Re: [PATCH net-next v7 06/10] net: openvswitch: store sampling probability in cb.

From: Aaron Conole
Date: Mon Jul 01 2024 - 14:25:11 EST


Adrian Moreno <amorenoz@xxxxxxxxxx> writes:

> When a packet sample is observed, the sampling rate that was used is
> important to estimate the real frequency of such event.
>
> Store the probability of the parent sample action in the skb's cb area
> and use it in psample action to pass it down to psample module.
>
> Acked-by: Eelco Chaudron <echaudro@xxxxxxxxxx>
> Reviewed-by: Ilya Maximets <i.maximets@xxxxxxx>
> Signed-off-by: Adrian Moreno <amorenoz@xxxxxxxxxx>
> ---

Reviewed-by: Aaron Conole <aconole@xxxxxxxxxx>

> include/uapi/linux/openvswitch.h | 3 ++-
> net/openvswitch/actions.c | 20 +++++++++++++++++---
> net/openvswitch/datapath.h | 3 +++
> net/openvswitch/vport.c | 1 +
> 4 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 3dd653748725..3a701bd1f31b 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -649,7 +649,8 @@ enum ovs_flow_attr {
> * Actions are passed as nested attributes.
> *
> * Executes the specified actions with the given probability on a per-packet
> - * basis.
> + * basis. Nested actions will be able to access the probability value of the
> + * parent @OVS_ACTION_ATTR_SAMPLE.
> */
> enum ovs_sample_attr {
> OVS_SAMPLE_ATTR_UNSPEC,
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index a035b7e677dd..34af6bce4085 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1048,12 +1048,15 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
> struct nlattr *sample_arg;
> int rem = nla_len(attr);
> const struct sample_arg *arg;
> + u32 init_probability;
> bool clone_flow_key;
> + int err;
>
> /* The first action is always 'OVS_SAMPLE_ATTR_ARG'. */
> sample_arg = nla_data(attr);
> arg = nla_data(sample_arg);
> actions = nla_next(sample_arg, &rem);
> + init_probability = OVS_CB(skb)->probability;
>
> if ((arg->probability != U32_MAX) &&
> (!arg->probability || get_random_u32() > arg->probability)) {
> @@ -1062,9 +1065,16 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
> return 0;
> }
>
> + OVS_CB(skb)->probability = arg->probability;
> +
> clone_flow_key = !arg->exec;
> - return clone_execute(dp, skb, key, 0, actions, rem, last,
> - clone_flow_key);
> + err = clone_execute(dp, skb, key, 0, actions, rem, last,
> + clone_flow_key);
> +
> + if (!last)
> + OVS_CB(skb)->probability = init_probability;
> +
> + return err;
> }
>
> /* When 'last' is true, clone() should always consume the 'skb'.
> @@ -1311,6 +1321,7 @@ static void execute_psample(struct datapath *dp, struct sk_buff *skb,
> struct psample_group psample_group = {};
> struct psample_metadata md = {};
> const struct nlattr *a;
> + u32 rate;
> int rem;
>
> nla_for_each_attr(a, nla_data(attr), nla_len(attr), rem) {
> @@ -1329,8 +1340,11 @@ static void execute_psample(struct datapath *dp, struct sk_buff *skb,
> psample_group.net = ovs_dp_get_net(dp);
> md.in_ifindex = OVS_CB(skb)->input_vport->dev->ifindex;
> md.trunc_size = skb->len - OVS_CB(skb)->cutlen;
> + md.rate_as_probability = 1;
> +
> + rate = OVS_CB(skb)->probability ? OVS_CB(skb)->probability : U32_MAX;
>
> - psample_sample_packet(&psample_group, skb, 0, &md);
> + psample_sample_packet(&psample_group, skb, rate, &md);
> }
> #else
> static inline void execute_psample(struct datapath *dp, struct sk_buff *skb,
> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> index 0cd29971a907..9ca6231ea647 100644
> --- a/net/openvswitch/datapath.h
> +++ b/net/openvswitch/datapath.h
> @@ -115,12 +115,15 @@ struct datapath {
> * fragmented.
> * @acts_origlen: The netlink size of the flow actions applied to this skb.
> * @cutlen: The number of bytes from the packet end to be removed.
> + * @probability: The sampling probability that was applied to this skb; 0 means
> + * no sampling has occurred; U32_MAX means 100% probability.
> */
> struct ovs_skb_cb {
> struct vport *input_vport;
> u16 mru;
> u16 acts_origlen;
> u32 cutlen;
> + u32 probability;
> };
> #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
>
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 972ae01a70f7..8732f6e51ae5 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -500,6 +500,7 @@ int ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
> OVS_CB(skb)->input_vport = vport;
> OVS_CB(skb)->mru = 0;
> OVS_CB(skb)->cutlen = 0;
> + OVS_CB(skb)->probability = 0;
> if (unlikely(dev_net(skb->dev) != ovs_dp_get_net(vport->dp))) {
> u32 mark;