| Message ID | 20240408125753.470419-4-amorenoz@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: openvswitch: Add sample multicasting. | expand |
[copying my previous reply since this version actually has netdev@ in Cc] On 4/8/24 14:57, Adrian Moreno wrote: > Add a user cookie to the sample metadata so that sample emitters can > provide more contextual information to samples. > > If present, send the user cookie in a new attribute: > PSAMPLE_ATTR_USER_COOKIE. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > include/net/psample.h | 2 ++ > include/uapi/linux/psample.h | 1 + > net/psample/psample.c | 12 +++++++++++- > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/net/psample.h b/include/net/psample.h > index 0509d2d6be67..2503ab3c92a5 100644 > --- a/include/net/psample.h > +++ b/include/net/psample.h > @@ -25,6 +25,8 @@ struct psample_metadata { > out_tc_occ_valid:1, > latency_valid:1, > unused:5; > + u8 *user_cookie; The code doesn't take ownership of this data. It should probably be a const pointer in this case. This should also allow us to avoid a double copy first to the psample_metadata and then to netlink skb, as users will know that it is a const pointer and so they can hand the data directly. > + u32 user_cookie_len; > }; > > struct psample_group *psample_group_get(struct net *net, u32 group_num); > diff --git a/include/uapi/linux/psample.h b/include/uapi/linux/psample.h > index 5e0305b1520d..1f61fd7ef7fd 100644 > --- a/include/uapi/linux/psample.h > +++ b/include/uapi/linux/psample.h > @@ -19,6 +19,7 @@ enum { > PSAMPLE_ATTR_LATENCY, /* u64, nanoseconds */ > PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */ > PSAMPLE_ATTR_PROTO, /* u16 */ > + PSAMPLE_ATTR_USER_COOKIE, A comment would be nice. > > __PSAMPLE_ATTR_MAX > }; > diff --git a/net/psample/psample.c b/net/psample/psample.c > index a0cef63dfdec..9fdb88e01f21 100644 > --- a/net/psample/psample.c > +++ b/net/psample/psample.c > @@ -497,7 +497,8 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, > nla_total_size(sizeof(u32)) + /* group_num */ > nla_total_size(sizeof(u32)) + /* seq */ > nla_total_size_64bit(sizeof(u64)) + /* timestamp */ > - nla_total_size(sizeof(u16)); /* protocol */ > + nla_total_size(sizeof(u16)) + /* protocol */ > + nla_total_size(md->user_cookie_len); /* user_cookie */ > > #ifdef CONFIG_INET > tun_info = skb_tunnel_info(skb); > @@ -596,6 +597,15 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, > goto error; > } > #endif > + if (md->user_cookie && md->user_cookie_len) { > + int nla_len = nla_total_size(md->user_cookie_len); > + struct nlattr *nla; > + > + nla = skb_put(nl_skb, nla_len); > + nla->nla_type = PSAMPLE_ATTR_USER_COOKIE; > + nla->nla_len = nla_attr_size(md->user_cookie_len); > + memcpy(nla_data(nla), md->user_cookie, md->user_cookie_len); > + } > > genlmsg_end(nl_skb, data); > psample_nl_obj_desc_init(&desc, group->group_num);
On 4/8/24 15:19, Ilya Maximets wrote: > [copying my previous reply since this version actually has netdev@ in Cc] > > On 4/8/24 14:57, Adrian Moreno wrote: >> Add a user cookie to the sample metadata so that sample emitters can >> provide more contextual information to samples. >> >> If present, send the user cookie in a new attribute: >> PSAMPLE_ATTR_USER_COOKIE. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> include/net/psample.h | 2 ++ >> include/uapi/linux/psample.h | 1 + >> net/psample/psample.c | 12 +++++++++++- >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/include/net/psample.h b/include/net/psample.h >> index 0509d2d6be67..2503ab3c92a5 100644 >> --- a/include/net/psample.h >> +++ b/include/net/psample.h >> @@ -25,6 +25,8 @@ struct psample_metadata { >> out_tc_occ_valid:1, >> latency_valid:1, >> unused:5; >> + u8 *user_cookie; > > The code doesn't take ownership of this data. It should probably > be a const pointer in this case. > > This should also allow us to avoid a double copy first to the > psample_metadata and then to netlink skb, as users will know that > it is a const pointer and so they can hand the data directly. > Yep. You're right. >> + u32 user_cookie_len; >> }; >> >> struct psample_group *psample_group_get(struct net *net, u32 group_num); >> diff --git a/include/uapi/linux/psample.h b/include/uapi/linux/psample.h >> index 5e0305b1520d..1f61fd7ef7fd 100644 >> --- a/include/uapi/linux/psample.h >> +++ b/include/uapi/linux/psample.h >> @@ -19,6 +19,7 @@ enum { >> PSAMPLE_ATTR_LATENCY, /* u64, nanoseconds */ >> PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */ >> PSAMPLE_ATTR_PROTO, /* u16 */ >> + PSAMPLE_ATTR_USER_COOKIE, > > A comment would be nice. > Definitely. >> >> __PSAMPLE_ATTR_MAX >> }; >> diff --git a/net/psample/psample.c b/net/psample/psample.c >> index a0cef63dfdec..9fdb88e01f21 100644 >> --- a/net/psample/psample.c >> +++ b/net/psample/psample.c >> @@ -497,7 +497,8 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, >> nla_total_size(sizeof(u32)) + /* group_num */ >> nla_total_size(sizeof(u32)) + /* seq */ >> nla_total_size_64bit(sizeof(u64)) + /* timestamp */ >> - nla_total_size(sizeof(u16)); /* protocol */ >> + nla_total_size(sizeof(u16)) + /* protocol */ >> + nla_total_size(md->user_cookie_len); /* user_cookie */ >> >> #ifdef CONFIG_INET >> tun_info = skb_tunnel_info(skb); >> @@ -596,6 +597,15 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, >> goto error; >> } >> #endif >> + if (md->user_cookie && md->user_cookie_len) { >> + int nla_len = nla_total_size(md->user_cookie_len); >> + struct nlattr *nla; >> + >> + nla = skb_put(nl_skb, nla_len); >> + nla->nla_type = PSAMPLE_ATTR_USER_COOKIE; >> + nla->nla_len = nla_attr_size(md->user_cookie_len); >> + memcpy(nla_data(nla), md->user_cookie, md->user_cookie_len); >> + } >> >> genlmsg_end(nl_skb, data); >> psample_nl_obj_desc_init(&desc, group->group_num); >
On 4/8/24 21:28, Adrian Moreno wrote: > > > On 4/8/24 15:19, Ilya Maximets wrote: >> [copying my previous reply since this version actually has netdev@ in Cc] >> >> On 4/8/24 14:57, Adrian Moreno wrote: >>> Add a user cookie to the sample metadata so that sample emitters can >>> provide more contextual information to samples. >>> >>> If present, send the user cookie in a new attribute: >>> PSAMPLE_ATTR_USER_COOKIE. >>> >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >>> --- >>> include/net/psample.h | 2 ++ >>> include/uapi/linux/psample.h | 1 + >>> net/psample/psample.c | 12 +++++++++++- >>> 3 files changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/net/psample.h b/include/net/psample.h >>> index 0509d2d6be67..2503ab3c92a5 100644 >>> --- a/include/net/psample.h >>> +++ b/include/net/psample.h >>> @@ -25,6 +25,8 @@ struct psample_metadata { >>> out_tc_occ_valid:1, >>> latency_valid:1, >>> unused:5; >>> + u8 *user_cookie; >> >> The code doesn't take ownership of this data. It should probably >> be a const pointer in this case. >> >> This should also allow us to avoid a double copy first to the >> psample_metadata and then to netlink skb, as users will know that >> it is a const pointer and so they can hand the data directly. >> > > Yep. You're right. > > >>> + u32 user_cookie_len; BTW, u32 looks a little strange here and by extension in tc and OVS as well. We can't push move than 64K through netlink, AFAIK, so u16 should be plenty here. In practice, I hope, we'll not go over u8 though. Best regards, Ilya Maximets.
diff --git a/include/net/psample.h b/include/net/psample.h index 0509d2d6be67..2503ab3c92a5 100644 --- a/include/net/psample.h +++ b/include/net/psample.h @@ -25,6 +25,8 @@ struct psample_metadata { out_tc_occ_valid:1, latency_valid:1, unused:5; + u8 *user_cookie; + u32 user_cookie_len; }; struct psample_group *psample_group_get(struct net *net, u32 group_num); diff --git a/include/uapi/linux/psample.h b/include/uapi/linux/psample.h index 5e0305b1520d..1f61fd7ef7fd 100644 --- a/include/uapi/linux/psample.h +++ b/include/uapi/linux/psample.h @@ -19,6 +19,7 @@ enum { PSAMPLE_ATTR_LATENCY, /* u64, nanoseconds */ PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */ PSAMPLE_ATTR_PROTO, /* u16 */ + PSAMPLE_ATTR_USER_COOKIE, __PSAMPLE_ATTR_MAX }; diff --git a/net/psample/psample.c b/net/psample/psample.c index a0cef63dfdec..9fdb88e01f21 100644 --- a/net/psample/psample.c +++ b/net/psample/psample.c @@ -497,7 +497,8 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, nla_total_size(sizeof(u32)) + /* group_num */ nla_total_size(sizeof(u32)) + /* seq */ nla_total_size_64bit(sizeof(u64)) + /* timestamp */ - nla_total_size(sizeof(u16)); /* protocol */ + nla_total_size(sizeof(u16)) + /* protocol */ + nla_total_size(md->user_cookie_len); /* user_cookie */ #ifdef CONFIG_INET tun_info = skb_tunnel_info(skb); @@ -596,6 +597,15 @@ void psample_sample_packet(struct psample_group *group, struct sk_buff *skb, goto error; } #endif + if (md->user_cookie && md->user_cookie_len) { + int nla_len = nla_total_size(md->user_cookie_len); + struct nlattr *nla; + + nla = skb_put(nl_skb, nla_len); + nla->nla_type = PSAMPLE_ATTR_USER_COOKIE; + nla->nla_len = nla_attr_size(md->user_cookie_len); + memcpy(nla_data(nla), md->user_cookie, md->user_cookie_len); + } genlmsg_end(nl_skb, data); psample_nl_obj_desc_init(&desc, group->group_num);
Add a user cookie to the sample metadata so that sample emitters can provide more contextual information to samples. If present, send the user cookie in a new attribute: PSAMPLE_ATTR_USER_COOKIE. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- include/net/psample.h | 2 ++ include/uapi/linux/psample.h | 1 + net/psample/psample.c | 12 +++++++++++- 3 files changed, 14 insertions(+), 1 deletion(-)