| Message ID | 20230425124122.2443376-6-roid@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add vxlan gbp offload with TC | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 25/04/2023 15:41, Roi Dayan wrote: > From: Gavin Li <gavinl@nvidia.com> > > Add TC offload support for filtering vxlan tunnels with gbp option > > Signed-off-by: Gavin Li <gavinl@nvidia.com> > Reviewed-by: Gavi Teitz <gavi@nvidia.com> > --- > include/linux/pkt_cls.h | 13 ++++++++++ > lib/netdev-offload-tc.c | 17 +++++++++++++ > lib/tc.c | 54 ++++++++++++++++++++++++++++++++++++++++- > lib/tc.h | 7 ++++++ > 4 files changed, 90 insertions(+), 1 deletion(-) > > diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h > index a8cd8db5bf88..fb4a7ecea4cc 100644 > --- a/include/linux/pkt_cls.h > +++ b/include/linux/pkt_cls.h > @@ -273,6 +273,10 @@ enum { > * TCA_TUNNEL_KEY_ENC_OPTS_GENEVE > * attributes > */ > + TCA_FLOWER_KEY_ENC_OPTS_VXLAN, /* Nested > + * TCA_TUNNEL_KEY_ENC_OPTS_VXLAN > + * attributes > + */ > __TCA_FLOWER_KEY_ENC_OPTS_MAX, > }; > > @@ -290,6 +294,15 @@ enum { > #define TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX \ > (__TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX - 1) > > +enum { > + TCA_FLOWER_KEY_ENC_OPT_VXLAN_UNSPEC, > + TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, /* u32 */ > + __TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX, > +}; > + > +#define TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX \ > + (__TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX - 1) > + > enum { > TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), > TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index c9662081fc60..ba4179cd8693 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -1230,6 +1230,15 @@ parse_tc_flower_to_match(const struct netdev *netdev, > match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst, > flower->mask.tunnel.tp_dst); > } > + if (flower->mask.tunnel.gbp.id) { > + match_set_tun_gbp_id_masked(match, flower->key.tunnel.gbp.id, > + flower->mask.tunnel.gbp.id); > + } > + if (flower->mask.tunnel.gbp.flags) { > + match_set_tun_gbp_flags_masked(match, > + flower->key.tunnel.gbp.flags, > + flower->mask.tunnel.gbp.flags); > + } > > if (!strcmp(netdev_get_type(netdev), "geneve")) { > flower_tun_opt_to_match(match, flower); > @@ -2189,6 +2198,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > flower.key.tunnel.ttl = tnl->ip_ttl; > flower.key.tunnel.tp_src = tnl->tp_src; > flower.key.tunnel.tp_dst = tnl->tp_dst; > + flower.key.tunnel.gbp.id = tnl->gbp_id; > + flower.key.tunnel.gbp.flags = tnl->gbp_flags; > + flower.key.tunnel.gbp.id_present = !!tnl_mask->gbp_id; > > flower.mask.tunnel.ipv4.ipv4_src = tnl_mask->ip_src; > flower.mask.tunnel.ipv4.ipv4_dst = tnl_mask->ip_dst; > @@ -2203,6 +2215,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > * Degrading the flow down to exact match for now as a workaround. */ > flower.mask.tunnel.tp_dst = OVS_BE16_MAX; > flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0; > + flower.mask.tunnel.gbp.id = tnl_mask->gbp_id; > + flower.mask.tunnel.gbp.flags = tnl_mask->gbp_flags; > + flower.mask.tunnel.gbp.id_present = !!tnl_mask->gbp_id; > > memset(&tnl_mask->ip_src, 0, sizeof tnl_mask->ip_src); > memset(&tnl_mask->ip_dst, 0, sizeof tnl_mask->ip_dst); > @@ -2214,6 +2229,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > memset(&tnl_mask->tp_dst, 0, sizeof tnl_mask->tp_dst); > > memset(&tnl_mask->tun_id, 0, sizeof tnl_mask->tun_id); > + memset(&tnl_mask->gbp_id, 0, sizeof tnl_mask->gbp_id); > + memset(&tnl_mask->gbp_flags, 0, sizeof tnl_mask->gbp_flags); > tnl_mask->flags &= ~FLOW_TNL_F_KEY; > > /* XXX: This is wrong! We're ignoring DF and CSUM flags configuration > diff --git a/lib/tc.c b/lib/tc.c > index 87615e979f1a..e72fa4235198 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -38,6 +38,7 @@ > #include "byte-order.h" > #include "netlink-socket.h" > #include "netlink.h" > +#include "odp-util.h" > #include "openvswitch/ofpbuf.h" > #include "openvswitch/util.h" > #include "openvswitch/vlog.h" > @@ -696,6 +697,38 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr, > return 0; > } > > +static int > +nl_parse_vxlan_key(const struct nlattr *in_nlattr, > + struct tc_flower_tunnel *tunnel) > +{ > + const struct ofpbuf *msg; > + struct nlattr *nla; > + struct ofpbuf buf; > + uint32_t gbp_raw; > + size_t left; > + > + nl_attr_get_nested(in_nlattr, &buf); > + msg = &buf; > + > + NL_ATTR_FOR_EACH (nla, left, ofpbuf_at(msg, 0, 0), msg->size) { > + uint16_t type = nl_attr_type(nla); > + > + switch (type) { > + case TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP: > + gbp_raw = nl_attr_get_u32(nla); > + odp_decode_gbp_raw(gbp_raw, &tunnel->gbp.id, > + &tunnel->gbp.flags); > + tunnel->gbp.id_present = true; > + break; > + default: > + VLOG_ERR_RL(&error_rl, "failed to parse vxlan tun options"); > + return EINVAL; > + } > + } > + > + return 0; > +} > + > static int > nl_parse_flower_tunnel_opts(struct nlattr *options, > struct tc_flower_tunnel *tunnel) > @@ -718,6 +751,13 @@ nl_parse_flower_tunnel_opts(struct nlattr *options, > return err; > } > > + break; > + case TCA_FLOWER_KEY_ENC_OPTS_VXLAN: > + err = nl_parse_vxlan_key(nla, tunnel); > + if (err) { > + return err; > + } > + > break; > } > } > @@ -3439,11 +3479,12 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, > int len, cnt = 0; > > len = metadata->present.len; > - if (!len) { > + if (!len && !tunnel->gbp.id_present) { > return; > } > > outer = nl_msg_start_nested(request, type); > + /* Geneve */ > while (len) { > opt = &metadata->opts.gnv[cnt]; > inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_GENEVE); > @@ -3459,6 +3500,17 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, > > nl_msg_end_nested(request, inner); > } > + > + /* VxLAN GBP */ > + if (tunnel->gbp.id_present) { > + uint32_t gbp_raw; > + > + gbp_raw = odp_encode_gbp_raw(tunnel->gbp.flags, tunnel->gbp.id); > + inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_VXLAN); > + > + nl_msg_put_u32(request, TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, gbp_raw); > + nl_msg_end_nested(request, inner); > + } > nl_msg_end_nested(request, outer); > } > > diff --git a/lib/tc.h b/lib/tc.h > index b9d449677ed9..95fff37b9b61 100644 > --- a/lib/tc.h > +++ b/lib/tc.h > @@ -105,6 +105,12 @@ struct tc_cookie { > size_t len; > }; > > +struct tc_tunnel_gbp { > + ovs_be16 id; > + uint8_t flags; > + bool id_present; > +}; > + > struct tc_flower_tunnel { > struct { > ovs_be32 ipv4_src; > @@ -118,6 +124,7 @@ struct tc_flower_tunnel { > uint8_t ttl; > ovs_be16 tp_src; > ovs_be16 tp_dst; > + struct tc_tunnel_gbp gbp; > ovs_be64 id; > struct tun_metadata metadata; > }; somehow missed the reviewed-by tag here. will be fixed if v3 is needed and for here just adding here. thanks Reviewed-by: Roi Dayan <roid@nvidia.com>
On Tue, Apr 25, 2023 at 03:41:20PM +0300, Roi Dayan via dev wrote: > From: Gavin Li <gavinl@nvidia.com> > > Add TC offload support for filtering vxlan tunnels with gbp option > > Signed-off-by: Gavin Li <gavinl@nvidia.com> > Reviewed-by: Gavi Teitz <gavi@nvidia.com> ... > @@ -3439,11 +3479,12 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, > int len, cnt = 0; > > len = metadata->present.len; > - if (!len) { > + if (!len && !tunnel->gbp.id_present) { > return; > } > > outer = nl_msg_start_nested(request, type); > + /* Geneve */ > while (len) { > opt = &metadata->opts.gnv[cnt]; > inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_GENEVE); > @@ -3459,6 +3500,17 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, > > nl_msg_end_nested(request, inner); > } I wonder if it would be slightly nicer to move the Geneve code above and the VxLAN GBP code below into helper functions that are called here. Also, I'm assuming this patch will be updated as per the discussion of attribute handling in patch 4/7. Other than these points this patch looks fine to me. > + > + /* VxLAN GBP */ > + if (tunnel->gbp.id_present) { > + uint32_t gbp_raw; > + > + gbp_raw = odp_encode_gbp_raw(tunnel->gbp.flags, tunnel->gbp.id); > + inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_VXLAN); > + > + nl_msg_put_u32(request, TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, gbp_raw); > + nl_msg_end_nested(request, inner); > + } > nl_msg_end_nested(request, outer); > } > ...
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h index a8cd8db5bf88..fb4a7ecea4cc 100644 --- a/include/linux/pkt_cls.h +++ b/include/linux/pkt_cls.h @@ -273,6 +273,10 @@ enum { * TCA_TUNNEL_KEY_ENC_OPTS_GENEVE * attributes */ + TCA_FLOWER_KEY_ENC_OPTS_VXLAN, /* Nested + * TCA_TUNNEL_KEY_ENC_OPTS_VXLAN + * attributes + */ __TCA_FLOWER_KEY_ENC_OPTS_MAX, }; @@ -290,6 +294,15 @@ enum { #define TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX \ (__TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX - 1) +enum { + TCA_FLOWER_KEY_ENC_OPT_VXLAN_UNSPEC, + TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, /* u32 */ + __TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX, +}; + +#define TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX \ + (__TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX - 1) + enum { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index c9662081fc60..ba4179cd8693 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -1230,6 +1230,15 @@ parse_tc_flower_to_match(const struct netdev *netdev, match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst, flower->mask.tunnel.tp_dst); } + if (flower->mask.tunnel.gbp.id) { + match_set_tun_gbp_id_masked(match, flower->key.tunnel.gbp.id, + flower->mask.tunnel.gbp.id); + } + if (flower->mask.tunnel.gbp.flags) { + match_set_tun_gbp_flags_masked(match, + flower->key.tunnel.gbp.flags, + flower->mask.tunnel.gbp.flags); + } if (!strcmp(netdev_get_type(netdev), "geneve")) { flower_tun_opt_to_match(match, flower); @@ -2189,6 +2198,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, flower.key.tunnel.ttl = tnl->ip_ttl; flower.key.tunnel.tp_src = tnl->tp_src; flower.key.tunnel.tp_dst = tnl->tp_dst; + flower.key.tunnel.gbp.id = tnl->gbp_id; + flower.key.tunnel.gbp.flags = tnl->gbp_flags; + flower.key.tunnel.gbp.id_present = !!tnl_mask->gbp_id; flower.mask.tunnel.ipv4.ipv4_src = tnl_mask->ip_src; flower.mask.tunnel.ipv4.ipv4_dst = tnl_mask->ip_dst; @@ -2203,6 +2215,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, * Degrading the flow down to exact match for now as a workaround. */ flower.mask.tunnel.tp_dst = OVS_BE16_MAX; flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0; + flower.mask.tunnel.gbp.id = tnl_mask->gbp_id; + flower.mask.tunnel.gbp.flags = tnl_mask->gbp_flags; + flower.mask.tunnel.gbp.id_present = !!tnl_mask->gbp_id; memset(&tnl_mask->ip_src, 0, sizeof tnl_mask->ip_src); memset(&tnl_mask->ip_dst, 0, sizeof tnl_mask->ip_dst); @@ -2214,6 +2229,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, memset(&tnl_mask->tp_dst, 0, sizeof tnl_mask->tp_dst); memset(&tnl_mask->tun_id, 0, sizeof tnl_mask->tun_id); + memset(&tnl_mask->gbp_id, 0, sizeof tnl_mask->gbp_id); + memset(&tnl_mask->gbp_flags, 0, sizeof tnl_mask->gbp_flags); tnl_mask->flags &= ~FLOW_TNL_F_KEY; /* XXX: This is wrong! We're ignoring DF and CSUM flags configuration diff --git a/lib/tc.c b/lib/tc.c index 87615e979f1a..e72fa4235198 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -38,6 +38,7 @@ #include "byte-order.h" #include "netlink-socket.h" #include "netlink.h" +#include "odp-util.h" #include "openvswitch/ofpbuf.h" #include "openvswitch/util.h" #include "openvswitch/vlog.h" @@ -696,6 +697,38 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr, return 0; } +static int +nl_parse_vxlan_key(const struct nlattr *in_nlattr, + struct tc_flower_tunnel *tunnel) +{ + const struct ofpbuf *msg; + struct nlattr *nla; + struct ofpbuf buf; + uint32_t gbp_raw; + size_t left; + + nl_attr_get_nested(in_nlattr, &buf); + msg = &buf; + + NL_ATTR_FOR_EACH (nla, left, ofpbuf_at(msg, 0, 0), msg->size) { + uint16_t type = nl_attr_type(nla); + + switch (type) { + case TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP: + gbp_raw = nl_attr_get_u32(nla); + odp_decode_gbp_raw(gbp_raw, &tunnel->gbp.id, + &tunnel->gbp.flags); + tunnel->gbp.id_present = true; + break; + default: + VLOG_ERR_RL(&error_rl, "failed to parse vxlan tun options"); + return EINVAL; + } + } + + return 0; +} + static int nl_parse_flower_tunnel_opts(struct nlattr *options, struct tc_flower_tunnel *tunnel) @@ -718,6 +751,13 @@ nl_parse_flower_tunnel_opts(struct nlattr *options, return err; } + break; + case TCA_FLOWER_KEY_ENC_OPTS_VXLAN: + err = nl_parse_vxlan_key(nla, tunnel); + if (err) { + return err; + } + break; } } @@ -3439,11 +3479,12 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, int len, cnt = 0; len = metadata->present.len; - if (!len) { + if (!len && !tunnel->gbp.id_present) { return; } outer = nl_msg_start_nested(request, type); + /* Geneve */ while (len) { opt = &metadata->opts.gnv[cnt]; inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_GENEVE); @@ -3459,6 +3500,17 @@ nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type, nl_msg_end_nested(request, inner); } + + /* VxLAN GBP */ + if (tunnel->gbp.id_present) { + uint32_t gbp_raw; + + gbp_raw = odp_encode_gbp_raw(tunnel->gbp.flags, tunnel->gbp.id); + inner = nl_msg_start_nested(request, TCA_FLOWER_KEY_ENC_OPTS_VXLAN); + + nl_msg_put_u32(request, TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, gbp_raw); + nl_msg_end_nested(request, inner); + } nl_msg_end_nested(request, outer); } diff --git a/lib/tc.h b/lib/tc.h index b9d449677ed9..95fff37b9b61 100644 --- a/lib/tc.h +++ b/lib/tc.h @@ -105,6 +105,12 @@ struct tc_cookie { size_t len; }; +struct tc_tunnel_gbp { + ovs_be16 id; + uint8_t flags; + bool id_present; +}; + struct tc_flower_tunnel { struct { ovs_be32 ipv4_src; @@ -118,6 +124,7 @@ struct tc_flower_tunnel { uint8_t ttl; ovs_be16 tp_src; ovs_be16 tp_dst; + struct tc_tunnel_gbp gbp; ovs_be64 id; struct tun_metadata metadata; };