| Message ID | 20220729145319.3228334-6-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tc: Fixes for tunnel offloading. | 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 2022-07-29 5:53 PM, Ilya Maximets wrote: > netdev_tc_flow_put() "consumes" the tunnel.tp_src value, but > it's never passed down to TC, and not parsed back. Fix that. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > lib/netdev-offload-tc.c | 6 ++++++ > lib/tc.c | 17 +++++++++++++++++ > 2 files changed, 23 insertions(+) > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 57385597a..f49efee1b 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -1114,6 +1114,10 @@ parse_tc_flower_to_match(struct tc_flower *flower, > match_set_tun_ttl_masked(match, flower->key.tunnel.ttl, > flower->mask.tunnel.ttl); > } > + if (flower->mask.tunnel.tp_src) { > + match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_src, > + flower->mask.tunnel.tp_src); > + } > if (flower->mask.tunnel.tp_dst) { > match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst, > flower->mask.tunnel.tp_dst); > @@ -2015,6 +2019,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > flower.mask.tunnel.ipv6.ipv6_dst = tnl_mask->ipv6_dst; > flower.mask.tunnel.tos = tnl_mask->ip_tos; > flower.mask.tunnel.ttl = tnl_mask->ip_ttl; > + flower.mask.tunnel.tp_src = tnl_mask->tp_src; > /* XXX: We should be setting the mask from 'tnl_mask->tp_dst' here, but > * some hardware drivers (mlx5) doesn't support masked matches and will > * refuse to offload such flows keeping them in software path. > @@ -2028,6 +2033,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > memset(&tnl_mask->ipv6_dst, 0, sizeof tnl_mask->ipv6_dst); > memset(&tnl_mask->ip_tos, 0, sizeof tnl_mask->ip_tos); > memset(&tnl_mask->ip_ttl, 0, sizeof tnl_mask->ip_ttl); > + memset(&tnl_mask->tp_src, 0, sizeof tnl_mask->tp_src); > memset(&tnl_mask->tp_dst, 0, sizeof tnl_mask->tp_dst); > > memset(&tnl_mask->tun_id, 0, sizeof tnl_mask->tun_id); > diff --git a/lib/tc.c b/lib/tc.c > index 7006e5e01..d099e4ab2 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -409,6 +409,10 @@ static const struct nl_policy tca_flower_policy[] = { > [TCA_FLOWER_KEY_ENC_IPV6_DST_MASK] = { .type = NL_A_UNSPEC, > .min_len = sizeof(struct in6_addr), > .optional = true, }, > + [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT] = { .type = NL_A_U16, > + .optional = true, }, > + [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK] = { .type = NL_A_U16, > + .optional = true, }, > [TCA_FLOWER_KEY_ENC_UDP_DST_PORT] = { .type = NL_A_U16, > .optional = true, }, > [TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK] = { .type = NL_A_U16, > @@ -783,6 +787,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) > flower->key.tunnel.ipv6.ipv6_dst = > nl_attr_get_in6_addr(attrs[TCA_FLOWER_KEY_ENC_IPV6_DST]); > } > + if (attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]) { > + flower->mask.tunnel.tp_src = > + nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]); > + flower->key.tunnel.tp_src = > + nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT]); > + } > if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]) { > flower->mask.tunnel.tp_dst = > nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]); > @@ -3393,12 +3403,14 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) > struct in6_addr *ipv6_src = &flower->key.tunnel.ipv6.ipv6_src; > struct in6_addr *ipv6_dst = &flower->key.tunnel.ipv6.ipv6_dst; > ovs_be32 id = be64_to_be32(flower->key.tunnel.id); > + ovs_be16 tp_src = flower->key.tunnel.tp_src; > ovs_be16 tp_dst = flower->key.tunnel.tp_dst; > uint8_t tos = flower->key.tunnel.tos; > uint8_t ttl = flower->key.tunnel.ttl; > uint8_t tos_mask = flower->mask.tunnel.tos; > uint8_t ttl_mask = flower->mask.tunnel.ttl; > ovs_be64 id_mask = flower->mask.tunnel.id; > + ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; > ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; > > if (ipv4_dst_mask || ipv4_src_mask) { > @@ -3425,6 +3437,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) > nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl); > nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask); > } > + if (tp_src_mask) { > + nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT, tp_src); > + nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK, > + tp_src_mask); > + } > if (tp_dst_mask) { > nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst); > nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, Reviewed-by: Roi Dayan <roid@nvidia.com>
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 57385597a..f49efee1b 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -1114,6 +1114,10 @@ parse_tc_flower_to_match(struct tc_flower *flower, match_set_tun_ttl_masked(match, flower->key.tunnel.ttl, flower->mask.tunnel.ttl); } + if (flower->mask.tunnel.tp_src) { + match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_src, + flower->mask.tunnel.tp_src); + } if (flower->mask.tunnel.tp_dst) { match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst, flower->mask.tunnel.tp_dst); @@ -2015,6 +2019,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, flower.mask.tunnel.ipv6.ipv6_dst = tnl_mask->ipv6_dst; flower.mask.tunnel.tos = tnl_mask->ip_tos; flower.mask.tunnel.ttl = tnl_mask->ip_ttl; + flower.mask.tunnel.tp_src = tnl_mask->tp_src; /* XXX: We should be setting the mask from 'tnl_mask->tp_dst' here, but * some hardware drivers (mlx5) doesn't support masked matches and will * refuse to offload such flows keeping them in software path. @@ -2028,6 +2033,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, memset(&tnl_mask->ipv6_dst, 0, sizeof tnl_mask->ipv6_dst); memset(&tnl_mask->ip_tos, 0, sizeof tnl_mask->ip_tos); memset(&tnl_mask->ip_ttl, 0, sizeof tnl_mask->ip_ttl); + memset(&tnl_mask->tp_src, 0, sizeof tnl_mask->tp_src); memset(&tnl_mask->tp_dst, 0, sizeof tnl_mask->tp_dst); memset(&tnl_mask->tun_id, 0, sizeof tnl_mask->tun_id); diff --git a/lib/tc.c b/lib/tc.c index 7006e5e01..d099e4ab2 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -409,6 +409,10 @@ static const struct nl_policy tca_flower_policy[] = { [TCA_FLOWER_KEY_ENC_IPV6_DST_MASK] = { .type = NL_A_UNSPEC, .min_len = sizeof(struct in6_addr), .optional = true, }, + [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT] = { .type = NL_A_U16, + .optional = true, }, + [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK] = { .type = NL_A_U16, + .optional = true, }, [TCA_FLOWER_KEY_ENC_UDP_DST_PORT] = { .type = NL_A_U16, .optional = true, }, [TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK] = { .type = NL_A_U16, @@ -783,6 +787,12 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) flower->key.tunnel.ipv6.ipv6_dst = nl_attr_get_in6_addr(attrs[TCA_FLOWER_KEY_ENC_IPV6_DST]); } + if (attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]) { + flower->mask.tunnel.tp_src = + nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]); + flower->key.tunnel.tp_src = + nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT]); + } if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]) { flower->mask.tunnel.tp_dst = nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]); @@ -3393,12 +3403,14 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) struct in6_addr *ipv6_src = &flower->key.tunnel.ipv6.ipv6_src; struct in6_addr *ipv6_dst = &flower->key.tunnel.ipv6.ipv6_dst; ovs_be32 id = be64_to_be32(flower->key.tunnel.id); + ovs_be16 tp_src = flower->key.tunnel.tp_src; ovs_be16 tp_dst = flower->key.tunnel.tp_dst; uint8_t tos = flower->key.tunnel.tos; uint8_t ttl = flower->key.tunnel.ttl; uint8_t tos_mask = flower->mask.tunnel.tos; uint8_t ttl_mask = flower->mask.tunnel.ttl; ovs_be64 id_mask = flower->mask.tunnel.id; + ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; if (ipv4_dst_mask || ipv4_src_mask) { @@ -3425,6 +3437,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl); nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask); } + if (tp_src_mask) { + nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT, tp_src); + nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK, + tp_src_mask); + } if (tp_dst_mask) { nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst); nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
netdev_tc_flow_put() "consumes" the tunnel.tp_src value, but it's never passed down to TC, and not parsed back. Fix that. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/netdev-offload-tc.c | 6 ++++++ lib/tc.c | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)