Hi Moty,
Please see few comments.
Tuesday, October 9, 2018 2:18 PM, Mordechay Haimovsky:
> Subject: [PATCH v2] net/mlx5: support e-switch TCP-flags flow filter
>
> This patch adds support for offloading flow rules with tcp-flags filter to
> mlx5
> eswitch hardware.
Hardware.
>
> Today, it is possible to offload an interface flow rules to the hardware using
> DPDK flow commands.
> With mlx5 it is also possible to offload a limited set of flow rules to the
> mlxsw
> (or e-switch) using the same DPDK flow commands.
This part Is not needed. You can say it is supported only under the transfer
attribute.
> A 'transfer' attribute was added to the flow rule creation command in order
> to distinguish between configuring port flows and E-switch flows.
> The commands destined for the switch are transposed to TC flower rules and
> are send, as Netlink messages, to the mlx5 driver, or more precisely to the
> netdev which represents the mlxsw port.
No need for the last phrase. Just say that this patch implement the TCP flags
under the transfer attr.
> The limited set of e-switch flow rules also supports filtering according to
> the
> values found in the TCP flags. This patch implements this offload capability
> in
> the mlx5 PMD.
>
> Signed-off-by: Moti Haimovsky <mo...@mellanox.com>
> ---
> v2:
> * Code rebase on latest upstream updates.
> ---
> drivers/net/mlx5/Makefile | 10 ++++++++++
> drivers/net/mlx5/mlx5_flow.c | 9 ++++++++-
> drivers/net/mlx5/mlx5_flow_tcf.c | 19 +++++++++++++++++++
> 3 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile index
> ca1de9f..9097456 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -342,6 +342,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-
> config-h.sh
> enum TCA_FLOWER_KEY_VLAN_ETH_TYPE \
> $(AUTOCONF_OUTPUT)
> $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_TCP_FLAGS \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_TCP_FLAGS \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
> + HAVE_TCA_FLOWER_KEY_TCP_FLAGS_MASK \
> + linux/pkt_cls.h \
> + enum TCA_FLOWER_KEY_TCP_FLAGS_MASK \
> + $(AUTOCONF_OUTPUT)
> + $Q sh -- '$<' '$@' \
Need to update also meson.build file.
> HAVE_TC_ACT_VLAN \
> linux/tc_act/tc_vlan.h \
> enum TCA_VLAN_PUSH_VLAN_PRIORITY \
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index ed60c40..c3dd060 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1239,6 +1239,13 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> struct rte_flow_error *error)
> {
> const struct rte_flow_item_tcp *mask = item->mask;
> + const struct rte_flow_item_tcp nic_mask = {
> + .hdr = {
> + .src_port = RTE_BE16(0xffff),
> + .dst_port = RTE_BE16(0xffff),
> + .tcp_flags = 0xff,
> + },
> + };
This validate function for TCP is shared between verbs, TC and DV flow engines.
The addition of the tcp_flags in this function would mean that verbs/dv also
support such filtering, while it is not the case today.
Maybe you should have different TCP validate function for tcf.
> const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
> int ret;
>
> @@ -1261,7 +1268,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> mask = &rte_flow_item_tcp_mask;
> ret = mlx5_flow_item_acceptable
> (item, (const uint8_t *)mask,
> - (const uint8_t *)&rte_flow_item_tcp_mask,
> + (const uint8_t *)&nic_mask,
> sizeof(struct rte_flow_item_tcp), error);
> if (ret < 0)
> return ret;
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 91f6ef6..d39ec2b 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -148,6 +148,12 @@ struct tc_vlan {
> #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE #define
> TCA_FLOWER_KEY_VLAN_ETH_TYPE 25 #endif
> +#ifndef HAVE_TCA_FLOWER_KEY_TCP_FLAGS
> +#define TCA_FLOWER_KEY_TCP_FLAGS 71
> +#endif
> +#ifndef HAVE_TCA_FLOWER_KEY_TCP_FLAGS_MASK
> +#define TCA_FLOWER_KEY_TCP_FLAGS_MASK 72 #endif
>
> #ifndef IPV6_ADDR_LEN
> #define IPV6_ADDR_LEN 16
> @@ -204,6 +210,7 @@ struct tc_vlan {
> .tcp.hdr = {
> .src_port = RTE_BE16(0xffff),
> .dst_port = RTE_BE16(0xffff),
> + .tcp_flags = 0xff,
> },
> .udp.hdr = {
> .src_port = RTE_BE16(0xffff),
> @@ -1289,6 +1296,18 @@ struct flow_tcf_ptoi {
>
> TCA_FLOWER_KEY_TCP_DST_MASK,
> mask.tcp->hdr.dst_port);
> }
> + if (mask.tcp->hdr.tcp_flags) {
> + mnl_attr_put_u16
> + (nlh,
> + TCA_FLOWER_KEY_TCP_FLAGS,
> + rte_cpu_to_be_16
> + (spec.tcp->hdr.tcp_flags));
> + mnl_attr_put_u16
> + (nlh,
> +
> TCA_FLOWER_KEY_TCP_FLAGS_MASK,
> + rte_cpu_to_be_16
> + (mask.tcp->hdr.tcp_flags));
> + }
> break;
> default:
> return rte_flow_error_set(error, ENOTSUP,
> --
> 1.8.3.1
