| Message ID | 33e81a4162a1349a50d029cdc74833da5b93c258.1728395410.git.echaudro@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add missing TC flower tunnel match and encap flags. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 08/10/2024 16:52, Eelco Chaudron wrote: > This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > acinclude.m4 | 6 +++--- > include/linux/pkt_cls.h | 28 ++++++++++++++++++++++++- > lib/netdev-offload-tc.c | 45 +++++++++++++++++++++++++++++++++++++++++ > lib/tc.c | 29 +++++++++++++++++++++++--- > lib/tc.h | 2 ++ > 5 files changed, 103 insertions(+), 7 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 1ace70c92..f4cb4a146 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -163,10 +163,10 @@ dnl Configure Linux tc compat. > AC_DEFUN([OVS_CHECK_LINUX_TC], [ > AC_COMPILE_IFELSE([ > AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [ > - int x = TCA_ACT_FLAGS_SKIP_HW; > + int x = TCA_FLOWER_KEY_ENC_FLAGS_MASK; > ])], > - [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1], > - [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])]) > + [AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK], [1], > + [Define to 1 if TCA_FLOWER_KEY_ENC_FLAGS_MASK is available.])]) > > AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include <linux/pkt_cls.h>]) > > diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h > index fb4a7ecea..5890dc157 100644 > --- a/include/linux/pkt_cls.h > +++ b/include/linux/pkt_cls.h > @@ -1,7 +1,7 @@ > #ifndef __LINUX_PKT_CLS_WRAPPER_H > #define __LINUX_PKT_CLS_WRAPPER_H 1 > > -#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW) > +#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK) > #include_next <linux/pkt_cls.h> > #else > > @@ -252,6 +252,28 @@ enum { > TCA_FLOWER_KEY_CT_LABELS, /* u128 */ > TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */ > > + TCA_FLOWER_KEY_MPLS_OPTS, > + > + TCA_FLOWER_KEY_HASH, /* u32 */ > + TCA_FLOWER_KEY_HASH_MASK, /* u32 */ > + > + TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ > + > + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ > + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ > + > + TCA_FLOWER_KEY_L2TPV3_SID, /* be32 */ > + > + TCA_FLOWER_L2_MISS, /* u8 */ > + > + TCA_FLOWER_KEY_CFM, /* nested */ > + > + TCA_FLOWER_KEY_SPI, /* be32 */ > + TCA_FLOWER_KEY_SPI_MASK, /* be32 */ > + > + TCA_FLOWER_KEY_ENC_FLAGS, /* be32 */ > + TCA_FLOWER_KEY_ENC_FLAGS_MASK, /* be32 */ > + > __TCA_FLOWER_MAX, > }; > > @@ -306,6 +328,10 @@ enum { > enum { > TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), > TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), > + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 2), > + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 3), > + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 4), > + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 5), > }; > > /* Match-all classifier */ > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 3be1c08d2..fcd206f2c 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -53,6 +53,7 @@ static bool multi_mask_per_prio = false; > static bool block_support = false; > static uint16_t ct_state_support; > static bool vxlan_gbp_support = false; > +static bool enc_flags_support = false; > > struct netlink_field { > int offset; > @@ -2863,6 +2864,49 @@ out: > tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); > } > > +static void > +probe_enc_flags_support(int ifindex) > +{ > + struct tc_flower flower; > + struct tcf_id id; > + int block_id = 0; > + int prio = TC_RESERVED_PRIORITY_FEATURE_PROBE; > + int error; > + > + error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS); > + if (error) { > + return; > + } > + > + memset(&flower, 0, sizeof flower); > + flower.tc_policy = TC_POLICY_SKIP_HW; > + flower.key.eth_type = htons(ETH_P_IP); > + flower.mask.eth_type = OVS_BE16_MAX; > + flower.tunnel = true; > + flower.mask.tunnel.id = OVS_BE64_MAX; > + flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX; > + flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX; > + flower.mask.tunnel.tp_dst = OVS_BE16_MAX; > + flower.mask.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; > + flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101); > + flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102); > + flower.key.tunnel.tp_dst = htons(46354); > + flower.key.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; > + > + id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS); > + error = tc_replace_flower(&id, &flower); > + if (error) { > + goto out; > + } > + > + tc_del_flower_filter(&id); > + > + enc_flags_support = true; > + VLOG_INFO("probe tc: enc flags are supported."); > +out: > + tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); > +} > + > static int > tc_get_policer_action_ids(struct hmap *map) > { > @@ -2991,6 +3035,7 @@ netdev_tc_init_flow_api(struct netdev *netdev) > probe_multi_mask_per_prio(ifindex); > probe_ct_state_support(ifindex); > probe_vxlan_gbp_support(ifindex); > + probe_enc_flags_support(ifindex); > > ovs_mutex_lock(&meter_police_ids_mutex); > meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE, > diff --git a/lib/tc.c b/lib/tc.c > index e55ba3b1b..6c853b8e6 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -454,6 +454,9 @@ static const struct nl_policy tca_flower_policy[] = { > [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, > [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NL_A_NESTED, > .optional = true, }, > + [TCA_FLOWER_KEY_ENC_FLAGS] = { .type = NL_A_BE32, .optional = true, }, > + [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = { .type = NL_A_BE32, > + .optional = true, }, > [TCA_FLOWER_KEY_CT_STATE] = { .type = NL_A_U16, .optional = true, }, > [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, }, > [TCA_FLOWER_KEY_CT_ZONE] = { .type = NL_A_U16, .optional = true, }, > @@ -865,6 +868,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) > flower->tunnel = true; > } > > + if (attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK]) { > + flower->key.tunnel.tc_enc_flags = ntohl( > + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS])); > + flower->mask.tunnel.tc_enc_flags = ntohl( > + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK])); > + } > + > if (attrs[TCA_FLOWER_KEY_ENC_OPTS] && > attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) { > err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS], > @@ -3611,6 +3621,7 @@ 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_be32 enc_flags = htonl(flower->key.tunnel.tc_enc_flags); > 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; > @@ -3618,6 +3629,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) > 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_be32 enc_flags_mask = htonl(flower->mask.tunnel.tc_enc_flags); > ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; > ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; > > @@ -3655,6 +3667,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) > nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, > tp_dst_mask); > } > + if (enc_flags_mask) { > + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS, enc_flags); > + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS_MASK, > + enc_flags_mask); > + } > if (id_mask) { > nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id); > } > @@ -3961,6 +3978,7 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) > struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size); > struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); > struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc); > + bool is_probe = id->prio == TC_RESERVED_PRIORITY_FEATURE_PROBE; > > if (!nlmsg || !tc) { > COVERAGE_INC(tc_netlink_malformed_reply); > @@ -3980,9 +3998,14 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) > false); > > if (ret || !cmp_tc_flower_match_action(flower, &flower_out)) { > - VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment does " > - "not match request! Set dpif_netlink to dbg to " > - "see which rule caused this error."); > + if (is_probe) { > + error = EINVAL; maybe can do that in another way, marking its a probe. see my comment about reserved prio below. > + } else { > + VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment " > + "does not match request! Set " > + "dpif_netlink to dbg to see " > + "which rule caused this error."); > + } > } > } > ofpbuf_delete(reply); > diff --git a/lib/tc.h b/lib/tc.h > index 8442c8d8b..8ec4857b7 100644 > --- a/lib/tc.h > +++ b/lib/tc.h > @@ -52,6 +52,7 @@ enum tc_flower_reserved_prio { > TC_RESERVED_PRIORITY_IPV4, > TC_RESERVED_PRIORITY_IPV6, > TC_RESERVED_PRIORITY_VLAN, > + TC_RESERVED_PRIORITY_FEATURE_PROBE, Hi, Why would you want to reserve a low priority for feature probe? The reserved priority was to make sure those types gets lower priority for better performance in HW offloading. so now one level will be saved for feature probing which is not needed and will never be used again after all probing was done. Unless I miss something I think we should not save a priority for feature probe. Thanks, Roi > __TC_RESERVED_PRIORITY_MAX > }; > #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1) > @@ -125,6 +126,7 @@ struct tc_flower_tunnel { > uint8_t ttl; > ovs_be16 tp_src; > ovs_be16 tp_dst; > + uint32_t tc_enc_flags; > struct tc_tunnel_gbp gbp; > ovs_be64 id; > struct tun_metadata metadata;
On 9 Oct 2024, at 8:50, Roi Dayan wrote: > On 08/10/2024 16:52, Eelco Chaudron wrote: >> This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported. >> >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> acinclude.m4 | 6 +++--- >> include/linux/pkt_cls.h | 28 ++++++++++++++++++++++++- >> lib/netdev-offload-tc.c | 45 +++++++++++++++++++++++++++++++++++++++++ >> lib/tc.c | 29 +++++++++++++++++++++++--- >> lib/tc.h | 2 ++ >> 5 files changed, 103 insertions(+), 7 deletions(-) >> >> diff --git a/acinclude.m4 b/acinclude.m4 >> index 1ace70c92..f4cb4a146 100644 >> --- a/acinclude.m4 >> +++ b/acinclude.m4 >> @@ -163,10 +163,10 @@ dnl Configure Linux tc compat. >> AC_DEFUN([OVS_CHECK_LINUX_TC], [ >> AC_COMPILE_IFELSE([ >> AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [ >> - int x = TCA_ACT_FLAGS_SKIP_HW; >> + int x = TCA_FLOWER_KEY_ENC_FLAGS_MASK; >> ])], >> - [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1], >> - [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])]) >> + [AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK], [1], >> + [Define to 1 if TCA_FLOWER_KEY_ENC_FLAGS_MASK is available.])]) >> >> AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include <linux/pkt_cls.h>]) >> >> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h >> index fb4a7ecea..5890dc157 100644 >> --- a/include/linux/pkt_cls.h >> +++ b/include/linux/pkt_cls.h >> @@ -1,7 +1,7 @@ >> #ifndef __LINUX_PKT_CLS_WRAPPER_H >> #define __LINUX_PKT_CLS_WRAPPER_H 1 >> >> -#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW) >> +#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK) >> #include_next <linux/pkt_cls.h> >> #else >> >> @@ -252,6 +252,28 @@ enum { >> TCA_FLOWER_KEY_CT_LABELS, /* u128 */ >> TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */ >> >> + TCA_FLOWER_KEY_MPLS_OPTS, >> + >> + TCA_FLOWER_KEY_HASH, /* u32 */ >> + TCA_FLOWER_KEY_HASH_MASK, /* u32 */ >> + >> + TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ >> + >> + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ >> + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ >> + >> + TCA_FLOWER_KEY_L2TPV3_SID, /* be32 */ >> + >> + TCA_FLOWER_L2_MISS, /* u8 */ >> + >> + TCA_FLOWER_KEY_CFM, /* nested */ >> + >> + TCA_FLOWER_KEY_SPI, /* be32 */ >> + TCA_FLOWER_KEY_SPI_MASK, /* be32 */ >> + >> + TCA_FLOWER_KEY_ENC_FLAGS, /* be32 */ >> + TCA_FLOWER_KEY_ENC_FLAGS_MASK, /* be32 */ >> + >> __TCA_FLOWER_MAX, >> }; >> >> @@ -306,6 +328,10 @@ enum { >> enum { >> TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), >> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), >> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 2), >> + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 3), >> + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 4), >> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 5), >> }; >> >> /* Match-all classifier */ >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >> index 3be1c08d2..fcd206f2c 100644 >> --- a/lib/netdev-offload-tc.c >> +++ b/lib/netdev-offload-tc.c >> @@ -53,6 +53,7 @@ static bool multi_mask_per_prio = false; >> static bool block_support = false; >> static uint16_t ct_state_support; >> static bool vxlan_gbp_support = false; >> +static bool enc_flags_support = false; >> >> struct netlink_field { >> int offset; >> @@ -2863,6 +2864,49 @@ out: >> tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >> } >> >> +static void >> +probe_enc_flags_support(int ifindex) >> +{ >> + struct tc_flower flower; >> + struct tcf_id id; >> + int block_id = 0; >> + int prio = TC_RESERVED_PRIORITY_FEATURE_PROBE; >> + int error; >> + >> + error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS); >> + if (error) { >> + return; >> + } >> + >> + memset(&flower, 0, sizeof flower); >> + flower.tc_policy = TC_POLICY_SKIP_HW; >> + flower.key.eth_type = htons(ETH_P_IP); >> + flower.mask.eth_type = OVS_BE16_MAX; >> + flower.tunnel = true; >> + flower.mask.tunnel.id = OVS_BE64_MAX; >> + flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX; >> + flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX; >> + flower.mask.tunnel.tp_dst = OVS_BE16_MAX; >> + flower.mask.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >> + flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101); >> + flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102); >> + flower.key.tunnel.tp_dst = htons(46354); >> + flower.key.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >> + >> + id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS); >> + error = tc_replace_flower(&id, &flower); >> + if (error) { >> + goto out; >> + } >> + >> + tc_del_flower_filter(&id); >> + >> + enc_flags_support = true; >> + VLOG_INFO("probe tc: enc flags are supported."); >> +out: >> + tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >> +} >> + >> static int >> tc_get_policer_action_ids(struct hmap *map) >> { >> @@ -2991,6 +3035,7 @@ netdev_tc_init_flow_api(struct netdev *netdev) >> probe_multi_mask_per_prio(ifindex); >> probe_ct_state_support(ifindex); >> probe_vxlan_gbp_support(ifindex); >> + probe_enc_flags_support(ifindex); >> >> ovs_mutex_lock(&meter_police_ids_mutex); >> meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE, >> diff --git a/lib/tc.c b/lib/tc.c >> index e55ba3b1b..6c853b8e6 100644 >> --- a/lib/tc.c >> +++ b/lib/tc.c >> @@ -454,6 +454,9 @@ static const struct nl_policy tca_flower_policy[] = { >> [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, >> [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NL_A_NESTED, >> .optional = true, }, >> + [TCA_FLOWER_KEY_ENC_FLAGS] = { .type = NL_A_BE32, .optional = true, }, >> + [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = { .type = NL_A_BE32, >> + .optional = true, }, >> [TCA_FLOWER_KEY_CT_STATE] = { .type = NL_A_U16, .optional = true, }, >> [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, }, >> [TCA_FLOWER_KEY_CT_ZONE] = { .type = NL_A_U16, .optional = true, }, >> @@ -865,6 +868,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) >> flower->tunnel = true; >> } >> >> + if (attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK]) { >> + flower->key.tunnel.tc_enc_flags = ntohl( >> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS])); >> + flower->mask.tunnel.tc_enc_flags = ntohl( >> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK])); >> + } >> + >> if (attrs[TCA_FLOWER_KEY_ENC_OPTS] && >> attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) { >> err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS], >> @@ -3611,6 +3621,7 @@ 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_be32 enc_flags = htonl(flower->key.tunnel.tc_enc_flags); >> 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; >> @@ -3618,6 +3629,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >> 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_be32 enc_flags_mask = htonl(flower->mask.tunnel.tc_enc_flags); >> ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; >> ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; >> >> @@ -3655,6 +3667,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >> nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, >> tp_dst_mask); >> } >> + if (enc_flags_mask) { >> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS, enc_flags); >> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS_MASK, >> + enc_flags_mask); >> + } >> if (id_mask) { >> nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id); >> } >> @@ -3961,6 +3978,7 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >> struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size); >> struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); >> struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc); >> + bool is_probe = id->prio == TC_RESERVED_PRIORITY_FEATURE_PROBE; >> >> if (!nlmsg || !tc) { >> COVERAGE_INC(tc_netlink_malformed_reply); >> @@ -3980,9 +3998,14 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >> false); >> >> if (ret || !cmp_tc_flower_match_action(flower, &flower_out)) { >> - VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment does " >> - "not match request! Set dpif_netlink to dbg to " >> - "see which rule caused this error."); >> + if (is_probe) { >> + error = EINVAL; > > maybe can do that in another way, marking its a probe. > see my comment about reserved prio below. > >> + } else { >> + VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment " >> + "does not match request! Set " >> + "dpif_netlink to dbg to see " >> + "which rule caused this error."); >> + } >> } >> } >> ofpbuf_delete(reply); >> diff --git a/lib/tc.h b/lib/tc.h >> index 8442c8d8b..8ec4857b7 100644 >> --- a/lib/tc.h >> +++ b/lib/tc.h >> @@ -52,6 +52,7 @@ enum tc_flower_reserved_prio { >> TC_RESERVED_PRIORITY_IPV4, >> TC_RESERVED_PRIORITY_IPV6, >> TC_RESERVED_PRIORITY_VLAN, >> + TC_RESERVED_PRIORITY_FEATURE_PROBE, > > Hi, > > Why would you want to reserve a low priority for feature probe? > The reserved priority was to make sure those types gets lower > priority for better performance in HW offloading. so now one > level will be saved for feature probing which is not needed > and will never be used again after all probing was done. > Unless I miss something I think we should not save a priority > for feature probe. Hi Roi, First, thanks for the review! Yes, there are alternative ways to handle this, but I felt using one of the 64K priorities would be a cleaner solution. As far as I know, we’ve never run into any issues with running out of them. We could add a flag to tc_replace_flower(), such as fail_match_action_cmp. Does anyone else have any other suggestions/opinions? If not, I’ll send out a v2 with this change, along with some include fixes to make the patch pass on GitHub. Cheers, Eelco >> __TC_RESERVED_PRIORITY_MAX >> }; >> #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1) >> @@ -125,6 +126,7 @@ struct tc_flower_tunnel { >> uint8_t ttl; >> ovs_be16 tp_src; >> ovs_be16 tp_dst; >> + uint32_t tc_enc_flags; >> struct tc_tunnel_gbp gbp; >> ovs_be64 id; >> struct tun_metadata metadata;
On 09/10/2024 11:27, Eelco Chaudron wrote: > > > On 9 Oct 2024, at 8:50, Roi Dayan wrote: > >> On 08/10/2024 16:52, Eelco Chaudron wrote: >>> This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported. >>> >>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >>> --- >>> acinclude.m4 | 6 +++--- >>> include/linux/pkt_cls.h | 28 ++++++++++++++++++++++++- >>> lib/netdev-offload-tc.c | 45 +++++++++++++++++++++++++++++++++++++++++ >>> lib/tc.c | 29 +++++++++++++++++++++++--- >>> lib/tc.h | 2 ++ >>> 5 files changed, 103 insertions(+), 7 deletions(-) >>> >>> diff --git a/acinclude.m4 b/acinclude.m4 >>> index 1ace70c92..f4cb4a146 100644 >>> --- a/acinclude.m4 >>> +++ b/acinclude.m4 >>> @@ -163,10 +163,10 @@ dnl Configure Linux tc compat. >>> AC_DEFUN([OVS_CHECK_LINUX_TC], [ >>> AC_COMPILE_IFELSE([ >>> AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [ >>> - int x = TCA_ACT_FLAGS_SKIP_HW; >>> + int x = TCA_FLOWER_KEY_ENC_FLAGS_MASK; >>> ])], >>> - [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1], >>> - [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])]) >>> + [AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK], [1], >>> + [Define to 1 if TCA_FLOWER_KEY_ENC_FLAGS_MASK is available.])]) >>> >>> AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include <linux/pkt_cls.h>]) >>> >>> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h >>> index fb4a7ecea..5890dc157 100644 >>> --- a/include/linux/pkt_cls.h >>> +++ b/include/linux/pkt_cls.h >>> @@ -1,7 +1,7 @@ >>> #ifndef __LINUX_PKT_CLS_WRAPPER_H >>> #define __LINUX_PKT_CLS_WRAPPER_H 1 >>> >>> -#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW) >>> +#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK) >>> #include_next <linux/pkt_cls.h> >>> #else >>> >>> @@ -252,6 +252,28 @@ enum { >>> TCA_FLOWER_KEY_CT_LABELS, /* u128 */ >>> TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */ >>> >>> + TCA_FLOWER_KEY_MPLS_OPTS, >>> + >>> + TCA_FLOWER_KEY_HASH, /* u32 */ >>> + TCA_FLOWER_KEY_HASH_MASK, /* u32 */ >>> + >>> + TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ >>> + >>> + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ >>> + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ >>> + >>> + TCA_FLOWER_KEY_L2TPV3_SID, /* be32 */ >>> + >>> + TCA_FLOWER_L2_MISS, /* u8 */ >>> + >>> + TCA_FLOWER_KEY_CFM, /* nested */ >>> + >>> + TCA_FLOWER_KEY_SPI, /* be32 */ >>> + TCA_FLOWER_KEY_SPI_MASK, /* be32 */ >>> + >>> + TCA_FLOWER_KEY_ENC_FLAGS, /* be32 */ >>> + TCA_FLOWER_KEY_ENC_FLAGS_MASK, /* be32 */ >>> + >>> __TCA_FLOWER_MAX, >>> }; >>> >>> @@ -306,6 +328,10 @@ enum { >>> enum { >>> TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), >>> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), >>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 2), >>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 3), >>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 4), >>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 5), >>> }; >>> >>> /* Match-all classifier */ >>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >>> index 3be1c08d2..fcd206f2c 100644 >>> --- a/lib/netdev-offload-tc.c >>> +++ b/lib/netdev-offload-tc.c >>> @@ -53,6 +53,7 @@ static bool multi_mask_per_prio = false; >>> static bool block_support = false; >>> static uint16_t ct_state_support; >>> static bool vxlan_gbp_support = false; >>> +static bool enc_flags_support = false; >>> >>> struct netlink_field { >>> int offset; >>> @@ -2863,6 +2864,49 @@ out: >>> tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >>> } >>> >>> +static void >>> +probe_enc_flags_support(int ifindex) >>> +{ >>> + struct tc_flower flower; >>> + struct tcf_id id; >>> + int block_id = 0; >>> + int prio = TC_RESERVED_PRIORITY_FEATURE_PROBE; >>> + int error; >>> + >>> + error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS); >>> + if (error) { >>> + return; >>> + } >>> + >>> + memset(&flower, 0, sizeof flower); >>> + flower.tc_policy = TC_POLICY_SKIP_HW; >>> + flower.key.eth_type = htons(ETH_P_IP); >>> + flower.mask.eth_type = OVS_BE16_MAX; >>> + flower.tunnel = true; >>> + flower.mask.tunnel.id = OVS_BE64_MAX; >>> + flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX; >>> + flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX; >>> + flower.mask.tunnel.tp_dst = OVS_BE16_MAX; >>> + flower.mask.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >>> + flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101); >>> + flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102); >>> + flower.key.tunnel.tp_dst = htons(46354); >>> + flower.key.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >>> + >>> + id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS); >>> + error = tc_replace_flower(&id, &flower); >>> + if (error) { >>> + goto out; >>> + } >>> + >>> + tc_del_flower_filter(&id); >>> + >>> + enc_flags_support = true; >>> + VLOG_INFO("probe tc: enc flags are supported."); >>> +out: >>> + tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >>> +} >>> + >>> static int >>> tc_get_policer_action_ids(struct hmap *map) >>> { >>> @@ -2991,6 +3035,7 @@ netdev_tc_init_flow_api(struct netdev *netdev) >>> probe_multi_mask_per_prio(ifindex); >>> probe_ct_state_support(ifindex); >>> probe_vxlan_gbp_support(ifindex); >>> + probe_enc_flags_support(ifindex); >>> >>> ovs_mutex_lock(&meter_police_ids_mutex); >>> meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE, >>> diff --git a/lib/tc.c b/lib/tc.c >>> index e55ba3b1b..6c853b8e6 100644 >>> --- a/lib/tc.c >>> +++ b/lib/tc.c >>> @@ -454,6 +454,9 @@ static const struct nl_policy tca_flower_policy[] = { >>> [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, >>> [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NL_A_NESTED, >>> .optional = true, }, >>> + [TCA_FLOWER_KEY_ENC_FLAGS] = { .type = NL_A_BE32, .optional = true, }, >>> + [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = { .type = NL_A_BE32, >>> + .optional = true, }, >>> [TCA_FLOWER_KEY_CT_STATE] = { .type = NL_A_U16, .optional = true, }, >>> [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, }, >>> [TCA_FLOWER_KEY_CT_ZONE] = { .type = NL_A_U16, .optional = true, }, >>> @@ -865,6 +868,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) >>> flower->tunnel = true; >>> } >>> >>> + if (attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK]) { >>> + flower->key.tunnel.tc_enc_flags = ntohl( >>> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS])); >>> + flower->mask.tunnel.tc_enc_flags = ntohl( >>> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK])); >>> + } >>> + >>> if (attrs[TCA_FLOWER_KEY_ENC_OPTS] && >>> attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) { >>> err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS], >>> @@ -3611,6 +3621,7 @@ 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_be32 enc_flags = htonl(flower->key.tunnel.tc_enc_flags); >>> 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; >>> @@ -3618,6 +3629,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >>> 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_be32 enc_flags_mask = htonl(flower->mask.tunnel.tc_enc_flags); >>> ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; >>> ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; >>> >>> @@ -3655,6 +3667,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >>> nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, >>> tp_dst_mask); >>> } >>> + if (enc_flags_mask) { >>> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS, enc_flags); >>> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS_MASK, >>> + enc_flags_mask); >>> + } >>> if (id_mask) { >>> nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id); >>> } >>> @@ -3961,6 +3978,7 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >>> struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size); >>> struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); >>> struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc); >>> + bool is_probe = id->prio == TC_RESERVED_PRIORITY_FEATURE_PROBE; >>> >>> if (!nlmsg || !tc) { >>> COVERAGE_INC(tc_netlink_malformed_reply); >>> @@ -3980,9 +3998,14 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >>> false); >>> >>> if (ret || !cmp_tc_flower_match_action(flower, &flower_out)) { >>> - VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment does " >>> - "not match request! Set dpif_netlink to dbg to " >>> - "see which rule caused this error."); >>> + if (is_probe) { >>> + error = EINVAL; >> >> maybe can do that in another way, marking its a probe. >> see my comment about reserved prio below. >> >>> + } else { >>> + VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment " >>> + "does not match request! Set " >>> + "dpif_netlink to dbg to see " >>> + "which rule caused this error."); >>> + } >>> } >>> } >>> ofpbuf_delete(reply); >>> diff --git a/lib/tc.h b/lib/tc.h >>> index 8442c8d8b..8ec4857b7 100644 >>> --- a/lib/tc.h >>> +++ b/lib/tc.h >>> @@ -52,6 +52,7 @@ enum tc_flower_reserved_prio { >>> TC_RESERVED_PRIORITY_IPV4, >>> TC_RESERVED_PRIORITY_IPV6, >>> TC_RESERVED_PRIORITY_VLAN, >>> + TC_RESERVED_PRIORITY_FEATURE_PROBE, >> >> Hi, >> >> Why would you want to reserve a low priority for feature probe? >> The reserved priority was to make sure those types gets lower >> priority for better performance in HW offloading. so now one >> level will be saved for feature probing which is not needed >> and will never be used again after all probing was done. >> Unless I miss something I think we should not save a priority >> for feature probe. > > Hi Roi, > > First, thanks for the review! Yes, there are alternative ways to handle this, but I felt using one of the 64K priorities would be a cleaner solution. As far as I know, we’ve never run into any issues with running out of them. > > We could add a flag to tc_replace_flower(), such as fail_match_action_cmp. Does anyone else have any other suggestions/opinions? If not, I’ll send out a v2 with this change, along with some include fixes to make the patch pass on GitHub. > > Cheers, > > Eelco Hi, The issue was not running out of prios but having lower prio for better performance as how hw handling the rule. So catching now a lower prio means all rules are by default added with higher prio. This will probably cause many changes later to save more prios before before feature_probe for different type of rules. The flag could also be the simple bool is_probe instead of the local bool you added. Another option is to save a prio for feature_probe as maybe the last possible prio and in get_next_available_prio() to account for that. For example we can define the following 2 and check for last prio instead of UINT16_MAX TC_RESERVED_PRIORITY_FEATURE_PROBE = UINT16_MAX TC_LAST_PRIO = TC_RESERVED_PRIORITY_FEATURE_PROBE - 1 if (last_prio == TC_LAST_PRIO) { return TC_RESERVED_PRIORITY_NONE; } In this case you dont need to add a flag and pass it froma ll callers now. what do you think? Thanks, Roi > > >>> __TC_RESERVED_PRIORITY_MAX >>> }; >>> #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1) >>> @@ -125,6 +126,7 @@ struct tc_flower_tunnel { >>> uint8_t ttl; >>> ovs_be16 tp_src; >>> ovs_be16 tp_dst; >>> + uint32_t tc_enc_flags; >>> struct tc_tunnel_gbp gbp; >>> ovs_be64 id; >>> struct tun_metadata metadata; >
On 9 Oct 2024, at 12:06, Roi Dayan wrote: > On 09/10/2024 11:27, Eelco Chaudron wrote: >> >> >> On 9 Oct 2024, at 8:50, Roi Dayan wrote: >> >>> On 08/10/2024 16:52, Eelco Chaudron wrote: >>>> This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported. >>>> >>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >>>> --- >>>> acinclude.m4 | 6 +++--- >>>> include/linux/pkt_cls.h | 28 ++++++++++++++++++++++++- >>>> lib/netdev-offload-tc.c | 45 +++++++++++++++++++++++++++++++++++++++++ >>>> lib/tc.c | 29 +++++++++++++++++++++++--- >>>> lib/tc.h | 2 ++ >>>> 5 files changed, 103 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/acinclude.m4 b/acinclude.m4 >>>> index 1ace70c92..f4cb4a146 100644 >>>> --- a/acinclude.m4 >>>> +++ b/acinclude.m4 >>>> @@ -163,10 +163,10 @@ dnl Configure Linux tc compat. >>>> AC_DEFUN([OVS_CHECK_LINUX_TC], [ >>>> AC_COMPILE_IFELSE([ >>>> AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [ >>>> - int x = TCA_ACT_FLAGS_SKIP_HW; >>>> + int x = TCA_FLOWER_KEY_ENC_FLAGS_MASK; >>>> ])], >>>> - [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1], >>>> - [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])]) >>>> + [AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK], [1], >>>> + [Define to 1 if TCA_FLOWER_KEY_ENC_FLAGS_MASK is available.])]) >>>> >>>> AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include <linux/pkt_cls.h>]) >>>> >>>> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h >>>> index fb4a7ecea..5890dc157 100644 >>>> --- a/include/linux/pkt_cls.h >>>> +++ b/include/linux/pkt_cls.h >>>> @@ -1,7 +1,7 @@ >>>> #ifndef __LINUX_PKT_CLS_WRAPPER_H >>>> #define __LINUX_PKT_CLS_WRAPPER_H 1 >>>> >>>> -#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW) >>>> +#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK) >>>> #include_next <linux/pkt_cls.h> >>>> #else >>>> >>>> @@ -252,6 +252,28 @@ enum { >>>> TCA_FLOWER_KEY_CT_LABELS, /* u128 */ >>>> TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */ >>>> >>>> + TCA_FLOWER_KEY_MPLS_OPTS, >>>> + >>>> + TCA_FLOWER_KEY_HASH, /* u32 */ >>>> + TCA_FLOWER_KEY_HASH_MASK, /* u32 */ >>>> + >>>> + TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ >>>> + >>>> + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ >>>> + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ >>>> + >>>> + TCA_FLOWER_KEY_L2TPV3_SID, /* be32 */ >>>> + >>>> + TCA_FLOWER_L2_MISS, /* u8 */ >>>> + >>>> + TCA_FLOWER_KEY_CFM, /* nested */ >>>> + >>>> + TCA_FLOWER_KEY_SPI, /* be32 */ >>>> + TCA_FLOWER_KEY_SPI_MASK, /* be32 */ >>>> + >>>> + TCA_FLOWER_KEY_ENC_FLAGS, /* be32 */ >>>> + TCA_FLOWER_KEY_ENC_FLAGS_MASK, /* be32 */ >>>> + >>>> __TCA_FLOWER_MAX, >>>> }; >>>> >>>> @@ -306,6 +328,10 @@ enum { >>>> enum { >>>> TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), >>>> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), >>>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 2), >>>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 3), >>>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 4), >>>> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 5), >>>> }; >>>> >>>> /* Match-all classifier */ >>>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >>>> index 3be1c08d2..fcd206f2c 100644 >>>> --- a/lib/netdev-offload-tc.c >>>> +++ b/lib/netdev-offload-tc.c >>>> @@ -53,6 +53,7 @@ static bool multi_mask_per_prio = false; >>>> static bool block_support = false; >>>> static uint16_t ct_state_support; >>>> static bool vxlan_gbp_support = false; >>>> +static bool enc_flags_support = false; >>>> >>>> struct netlink_field { >>>> int offset; >>>> @@ -2863,6 +2864,49 @@ out: >>>> tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >>>> } >>>> >>>> +static void >>>> +probe_enc_flags_support(int ifindex) >>>> +{ >>>> + struct tc_flower flower; >>>> + struct tcf_id id; >>>> + int block_id = 0; >>>> + int prio = TC_RESERVED_PRIORITY_FEATURE_PROBE; >>>> + int error; >>>> + >>>> + error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS); >>>> + if (error) { >>>> + return; >>>> + } >>>> + >>>> + memset(&flower, 0, sizeof flower); >>>> + flower.tc_policy = TC_POLICY_SKIP_HW; >>>> + flower.key.eth_type = htons(ETH_P_IP); >>>> + flower.mask.eth_type = OVS_BE16_MAX; >>>> + flower.tunnel = true; >>>> + flower.mask.tunnel.id = OVS_BE64_MAX; >>>> + flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX; >>>> + flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX; >>>> + flower.mask.tunnel.tp_dst = OVS_BE16_MAX; >>>> + flower.mask.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >>>> + flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101); >>>> + flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102); >>>> + flower.key.tunnel.tp_dst = htons(46354); >>>> + flower.key.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; >>>> + >>>> + id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS); >>>> + error = tc_replace_flower(&id, &flower); >>>> + if (error) { >>>> + goto out; >>>> + } >>>> + >>>> + tc_del_flower_filter(&id); >>>> + >>>> + enc_flags_support = true; >>>> + VLOG_INFO("probe tc: enc flags are supported."); >>>> +out: >>>> + tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); >>>> +} >>>> + >>>> static int >>>> tc_get_policer_action_ids(struct hmap *map) >>>> { >>>> @@ -2991,6 +3035,7 @@ netdev_tc_init_flow_api(struct netdev *netdev) >>>> probe_multi_mask_per_prio(ifindex); >>>> probe_ct_state_support(ifindex); >>>> probe_vxlan_gbp_support(ifindex); >>>> + probe_enc_flags_support(ifindex); >>>> >>>> ovs_mutex_lock(&meter_police_ids_mutex); >>>> meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE, >>>> diff --git a/lib/tc.c b/lib/tc.c >>>> index e55ba3b1b..6c853b8e6 100644 >>>> --- a/lib/tc.c >>>> +++ b/lib/tc.c >>>> @@ -454,6 +454,9 @@ static const struct nl_policy tca_flower_policy[] = { >>>> [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, >>>> [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NL_A_NESTED, >>>> .optional = true, }, >>>> + [TCA_FLOWER_KEY_ENC_FLAGS] = { .type = NL_A_BE32, .optional = true, }, >>>> + [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = { .type = NL_A_BE32, >>>> + .optional = true, }, >>>> [TCA_FLOWER_KEY_CT_STATE] = { .type = NL_A_U16, .optional = true, }, >>>> [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, }, >>>> [TCA_FLOWER_KEY_CT_ZONE] = { .type = NL_A_U16, .optional = true, }, >>>> @@ -865,6 +868,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) >>>> flower->tunnel = true; >>>> } >>>> >>>> + if (attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK]) { >>>> + flower->key.tunnel.tc_enc_flags = ntohl( >>>> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS])); >>>> + flower->mask.tunnel.tc_enc_flags = ntohl( >>>> + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK])); >>>> + } >>>> + >>>> if (attrs[TCA_FLOWER_KEY_ENC_OPTS] && >>>> attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) { >>>> err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS], >>>> @@ -3611,6 +3621,7 @@ 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_be32 enc_flags = htonl(flower->key.tunnel.tc_enc_flags); >>>> 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; >>>> @@ -3618,6 +3629,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >>>> 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_be32 enc_flags_mask = htonl(flower->mask.tunnel.tc_enc_flags); >>>> ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; >>>> ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; >>>> >>>> @@ -3655,6 +3667,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) >>>> nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, >>>> tp_dst_mask); >>>> } >>>> + if (enc_flags_mask) { >>>> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS, enc_flags); >>>> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS_MASK, >>>> + enc_flags_mask); >>>> + } >>>> if (id_mask) { >>>> nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id); >>>> } >>>> @@ -3961,6 +3978,7 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >>>> struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size); >>>> struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); >>>> struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc); >>>> + bool is_probe = id->prio == TC_RESERVED_PRIORITY_FEATURE_PROBE; >>>> >>>> if (!nlmsg || !tc) { >>>> COVERAGE_INC(tc_netlink_malformed_reply); >>>> @@ -3980,9 +3998,14 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) >>>> false); >>>> >>>> if (ret || !cmp_tc_flower_match_action(flower, &flower_out)) { >>>> - VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment does " >>>> - "not match request! Set dpif_netlink to dbg to " >>>> - "see which rule caused this error."); >>>> + if (is_probe) { >>>> + error = EINVAL; >>> >>> maybe can do that in another way, marking its a probe. >>> see my comment about reserved prio below. >>> >>>> + } else { >>>> + VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment " >>>> + "does not match request! Set " >>>> + "dpif_netlink to dbg to see " >>>> + "which rule caused this error."); >>>> + } >>>> } >>>> } >>>> ofpbuf_delete(reply); >>>> diff --git a/lib/tc.h b/lib/tc.h >>>> index 8442c8d8b..8ec4857b7 100644 >>>> --- a/lib/tc.h >>>> +++ b/lib/tc.h >>>> @@ -52,6 +52,7 @@ enum tc_flower_reserved_prio { >>>> TC_RESERVED_PRIORITY_IPV4, >>>> TC_RESERVED_PRIORITY_IPV6, >>>> TC_RESERVED_PRIORITY_VLAN, >>>> + TC_RESERVED_PRIORITY_FEATURE_PROBE, >>> >>> Hi, >>> >>> Why would you want to reserve a low priority for feature probe? >>> The reserved priority was to make sure those types gets lower >>> priority for better performance in HW offloading. so now one >>> level will be saved for feature probing which is not needed >>> and will never be used again after all probing was done. >>> Unless I miss something I think we should not save a priority >>> for feature probe. >> >> Hi Roi, >> >> First, thanks for the review! Yes, there are alternative ways to handle this, but I felt using one of the 64K priorities would be a cleaner solution. As far as I know, we’ve never run into any issues with running out of them. >> >> We could add a flag to tc_replace_flower(), such as fail_match_action_cmp. Does anyone else have any other suggestions/opinions? If not, I’ll send out a v2 with this change, along with some include fixes to make the patch pass on GitHub. >> >> Cheers, >> >> Eelco > > Hi, > > The issue was not running out of prios but having lower prio > for better performance as how hw handling the rule. > So catching now a lower prio means all rules are by default added > with higher prio. > This will probably cause many changes later to save more prios before > before feature_probe for different type of rules. > > The flag could also be the simple bool is_probe instead of the local > bool you added. > > Another option is to save a prio for feature_probe as maybe the last > possible prio and in get_next_available_prio() to account for that. > For example we can define the following 2 and check for last prio instead of UINT16_MAX > > > TC_RESERVED_PRIORITY_FEATURE_PROBE = UINT16_MAX > TC_LAST_PRIO = TC_RESERVED_PRIORITY_FEATURE_PROBE - 1 > > if (last_prio == TC_LAST_PRIO) { > return TC_RESERVED_PRIORITY_NONE; > } > > In this case you dont need to add a flag and pass it froma ll callers now. > what do you think? I like this idea :) Let me think about it a bit more, and I will send out a v2 next week. >>>> __TC_RESERVED_PRIORITY_MAX >>>> }; >>>> #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1) >>>> @@ -125,6 +126,7 @@ struct tc_flower_tunnel { >>>> uint8_t ttl; >>>> ovs_be16 tp_src; >>>> ovs_be16 tp_dst; >>>> + uint32_t tc_enc_flags; >>>> struct tc_tunnel_gbp gbp; >>>> ovs_be64 id; >>>> struct tun_metadata metadata; >>
diff --git a/acinclude.m4 b/acinclude.m4 index 1ace70c92..f4cb4a146 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -163,10 +163,10 @@ dnl Configure Linux tc compat. AC_DEFUN([OVS_CHECK_LINUX_TC], [ AC_COMPILE_IFELSE([ AC_LANG_PROGRAM([#include <linux/pkt_cls.h>], [ - int x = TCA_ACT_FLAGS_SKIP_HW; + int x = TCA_FLOWER_KEY_ENC_FLAGS_MASK; ])], - [AC_DEFINE([HAVE_TCA_ACT_FLAGS_SKIP_HW], [1], - [Define to 1 if TCA_ACT_FLAGS_SKIP_HW is available.])]) + [AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK], [1], + [Define to 1 if TCA_FLOWER_KEY_ENC_FLAGS_MASK is available.])]) AC_CHECK_MEMBERS([struct tcf_t.firstuse], [], [], [#include <linux/pkt_cls.h>]) diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h index fb4a7ecea..5890dc157 100644 --- a/include/linux/pkt_cls.h +++ b/include/linux/pkt_cls.h @@ -1,7 +1,7 @@ #ifndef __LINUX_PKT_CLS_WRAPPER_H #define __LINUX_PKT_CLS_WRAPPER_H 1 -#if defined(__KERNEL__) || defined(HAVE_TCA_ACT_FLAGS_SKIP_HW) +#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_FLAGS_MASK) #include_next <linux/pkt_cls.h> #else @@ -252,6 +252,28 @@ enum { TCA_FLOWER_KEY_CT_LABELS, /* u128 */ TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */ + TCA_FLOWER_KEY_MPLS_OPTS, + + TCA_FLOWER_KEY_HASH, /* u32 */ + TCA_FLOWER_KEY_HASH_MASK, /* u32 */ + + TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ + + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ + + TCA_FLOWER_KEY_L2TPV3_SID, /* be32 */ + + TCA_FLOWER_L2_MISS, /* u8 */ + + TCA_FLOWER_KEY_CFM, /* nested */ + + TCA_FLOWER_KEY_SPI, /* be32 */ + TCA_FLOWER_KEY_SPI_MASK, /* be32 */ + + TCA_FLOWER_KEY_ENC_FLAGS, /* be32 */ + TCA_FLOWER_KEY_ENC_FLAGS_MASK, /* be32 */ + __TCA_FLOWER_MAX, }; @@ -306,6 +328,10 @@ enum { enum { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0), TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 2), + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 3), + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 4), + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 5), }; /* Match-all classifier */ diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 3be1c08d2..fcd206f2c 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -53,6 +53,7 @@ static bool multi_mask_per_prio = false; static bool block_support = false; static uint16_t ct_state_support; static bool vxlan_gbp_support = false; +static bool enc_flags_support = false; struct netlink_field { int offset; @@ -2863,6 +2864,49 @@ out: tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); } +static void +probe_enc_flags_support(int ifindex) +{ + struct tc_flower flower; + struct tcf_id id; + int block_id = 0; + int prio = TC_RESERVED_PRIORITY_FEATURE_PROBE; + int error; + + error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS); + if (error) { + return; + } + + memset(&flower, 0, sizeof flower); + flower.tc_policy = TC_POLICY_SKIP_HW; + flower.key.eth_type = htons(ETH_P_IP); + flower.mask.eth_type = OVS_BE16_MAX; + flower.tunnel = true; + flower.mask.tunnel.id = OVS_BE64_MAX; + flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX; + flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX; + flower.mask.tunnel.tp_dst = OVS_BE16_MAX; + flower.mask.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; + flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101); + flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102); + flower.key.tunnel.tp_dst = htons(46354); + flower.key.tunnel.tc_enc_flags = TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT; + + id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS); + error = tc_replace_flower(&id, &flower); + if (error) { + goto out; + } + + tc_del_flower_filter(&id); + + enc_flags_support = true; + VLOG_INFO("probe tc: enc flags are supported."); +out: + tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS); +} + static int tc_get_policer_action_ids(struct hmap *map) { @@ -2991,6 +3035,7 @@ netdev_tc_init_flow_api(struct netdev *netdev) probe_multi_mask_per_prio(ifindex); probe_ct_state_support(ifindex); probe_vxlan_gbp_support(ifindex); + probe_enc_flags_support(ifindex); ovs_mutex_lock(&meter_police_ids_mutex); meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE, diff --git a/lib/tc.c b/lib/tc.c index e55ba3b1b..6c853b8e6 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -454,6 +454,9 @@ static const struct nl_policy tca_flower_policy[] = { [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, }, [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NL_A_NESTED, .optional = true, }, + [TCA_FLOWER_KEY_ENC_FLAGS] = { .type = NL_A_BE32, .optional = true, }, + [TCA_FLOWER_KEY_ENC_FLAGS_MASK] = { .type = NL_A_BE32, + .optional = true, }, [TCA_FLOWER_KEY_CT_STATE] = { .type = NL_A_U16, .optional = true, }, [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NL_A_U16, .optional = true, }, [TCA_FLOWER_KEY_CT_ZONE] = { .type = NL_A_U16, .optional = true, }, @@ -865,6 +868,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower) flower->tunnel = true; } + if (attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK]) { + flower->key.tunnel.tc_enc_flags = ntohl( + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS])); + flower->mask.tunnel.tc_enc_flags = ntohl( + nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_FLAGS_MASK])); + } + if (attrs[TCA_FLOWER_KEY_ENC_OPTS] && attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) { err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS], @@ -3611,6 +3621,7 @@ 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_be32 enc_flags = htonl(flower->key.tunnel.tc_enc_flags); 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; @@ -3618,6 +3629,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) 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_be32 enc_flags_mask = htonl(flower->mask.tunnel.tc_enc_flags); ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src; ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst; @@ -3655,6 +3667,11 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower) nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, tp_dst_mask); } + if (enc_flags_mask) { + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS, enc_flags); + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_FLAGS_MASK, + enc_flags_mask); + } if (id_mask) { nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id); } @@ -3961,6 +3978,7 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size); struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc); + bool is_probe = id->prio == TC_RESERVED_PRIORITY_FEATURE_PROBE; if (!nlmsg || !tc) { COVERAGE_INC(tc_netlink_malformed_reply); @@ -3980,9 +3998,14 @@ tc_replace_flower(struct tcf_id *id, struct tc_flower *flower) false); if (ret || !cmp_tc_flower_match_action(flower, &flower_out)) { - VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment does " - "not match request! Set dpif_netlink to dbg to " - "see which rule caused this error."); + if (is_probe) { + error = EINVAL; + } else { + VLOG_WARN_RL(&error_rl, "Kernel flower acknowledgment " + "does not match request! Set " + "dpif_netlink to dbg to see " + "which rule caused this error."); + } } } ofpbuf_delete(reply); diff --git a/lib/tc.h b/lib/tc.h index 8442c8d8b..8ec4857b7 100644 --- a/lib/tc.h +++ b/lib/tc.h @@ -52,6 +52,7 @@ enum tc_flower_reserved_prio { TC_RESERVED_PRIORITY_IPV4, TC_RESERVED_PRIORITY_IPV6, TC_RESERVED_PRIORITY_VLAN, + TC_RESERVED_PRIORITY_FEATURE_PROBE, __TC_RESERVED_PRIORITY_MAX }; #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1) @@ -125,6 +126,7 @@ struct tc_flower_tunnel { uint8_t ttl; ovs_be16 tp_src; ovs_be16 tp_dst; + uint32_t tc_enc_flags; struct tc_tunnel_gbp gbp; ovs_be64 id; struct tun_metadata metadata;
This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- acinclude.m4 | 6 +++--- include/linux/pkt_cls.h | 28 ++++++++++++++++++++++++- lib/netdev-offload-tc.c | 45 +++++++++++++++++++++++++++++++++++++++++ lib/tc.c | 29 +++++++++++++++++++++++--- lib/tc.h | 2 ++ 5 files changed, 103 insertions(+), 7 deletions(-)