[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f7tjzi5vukx.fsf@redhat.com>
Date: Mon, 01 Jul 2024 14:24:46 -0400
From: Aaron Conole <aconole@...hat.com>
To: Adrian Moreno <amorenoz@...hat.com>
Cc: netdev@...r.kernel.org, echaudro@...hat.com, horms@...nel.org,
i.maximets@....org, dev@...nvswitch.org, Pravin B Shelar
<pshelar@....org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v7 06/10] net: openvswitch: store sampling
probability in cb.
Adrian Moreno <amorenoz@...hat.com> 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@...hat.com>
> Reviewed-by: Ilya Maximets <i.maximets@....org>
> Signed-off-by: Adrian Moreno <amorenoz@...hat.com>
> ---
Reviewed-by: Aaron Conole <aconole@...hat.com>
> 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;
Powered by blists - more mailing lists