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 <[email protected]>
> ---
> 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;
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
