From: Yongseok Koh <yskoh@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>,
Nelio Laranjeiro <nelio.laranjeiro@6wind.com>,
dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 5/6] net/mlx5: add VLAN item and actions to switch flow rules
Date: Wed, 11 Jul 2018 18:10:25 -0700 [thread overview]
Message-ID: <20180712011024.GG69686@yongseok-MBP.local> (raw)
In-Reply-To: <20180627173355.4718-6-adrien.mazarguil@6wind.com>
On Wed, Jun 27, 2018 at 08:08:18PM +0200, Adrien Mazarguil wrote:
> This enables flow rules to explicitly match VLAN traffic (VLAN pattern
> item) and perform various operations on VLAN headers at the switch level
> (OF_POP_VLAN, OF_PUSH_VLAN, OF_SET_VLAN_VID and OF_SET_VLAN_PCP actions).
>
> Testpmd examples:
>
> - Directing all VLAN traffic received on port ID 1 to port ID 0:
>
> flow create 1 ingress transfer pattern eth / vlan / end actions
> port_id id 0 / end
>
> - Adding a VLAN header to IPv6 traffic received on port ID 1 and directing
> it to port ID 0:
>
> flow create 1 ingress transfer pattern eth / ipv6 / end actions
> of_push_vlan ethertype 0x8100 / of_set_vlan_vid / port_id id 0 / end
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> ---
> drivers/net/mlx5/mlx5_nl_flow.c | 177 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 173 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_nl_flow.c b/drivers/net/mlx5/mlx5_nl_flow.c
> index ad1e001c6..a45d94fae 100644
> --- a/drivers/net/mlx5/mlx5_nl_flow.c
> +++ b/drivers/net/mlx5/mlx5_nl_flow.c
> @@ -13,6 +13,7 @@
> #include <linux/rtnetlink.h>
> #include <linux/tc_act/tc_gact.h>
> #include <linux/tc_act/tc_mirred.h>
> +#include <linux/tc_act/tc_vlan.h>
> #include <netinet/in.h>
> #include <stdalign.h>
> #include <stdbool.h>
> @@ -36,6 +37,7 @@ enum mlx5_nl_flow_trans {
> PATTERN,
> ITEM_VOID,
> ITEM_ETH,
> + ITEM_VLAN,
> ITEM_IPV4,
> ITEM_IPV6,
> ITEM_TCP,
> @@ -44,6 +46,10 @@ enum mlx5_nl_flow_trans {
> ACTION_VOID,
> ACTION_PORT_ID,
> ACTION_DROP,
> + ACTION_OF_POP_VLAN,
> + ACTION_OF_PUSH_VLAN,
> + ACTION_OF_SET_VLAN_VID,
> + ACTION_OF_SET_VLAN_PCP,
> END,
> };
>
> @@ -52,7 +58,8 @@ enum mlx5_nl_flow_trans {
> #define PATTERN_COMMON \
> ITEM_VOID, ACTIONS
> #define ACTIONS_COMMON \
> - ACTION_VOID
> + ACTION_VOID, ACTION_OF_POP_VLAN, ACTION_OF_PUSH_VLAN, \
> + ACTION_OF_SET_VLAN_VID, ACTION_OF_SET_VLAN_PCP
> #define ACTIONS_FATE \
> ACTION_PORT_ID, ACTION_DROP
>
> @@ -63,7 +70,8 @@ static const enum mlx5_nl_flow_trans *const mlx5_nl_flow_trans[] = {
> [ATTR] = TRANS(PATTERN),
> [PATTERN] = TRANS(ITEM_ETH, PATTERN_COMMON),
> [ITEM_VOID] = TRANS(BACK),
> - [ITEM_ETH] = TRANS(ITEM_IPV4, ITEM_IPV6, PATTERN_COMMON),
> + [ITEM_ETH] = TRANS(ITEM_IPV4, ITEM_IPV6, ITEM_VLAN, PATTERN_COMMON),
> + [ITEM_VLAN] = TRANS(ITEM_IPV4, ITEM_IPV6, PATTERN_COMMON),
> [ITEM_IPV4] = TRANS(ITEM_TCP, ITEM_UDP, PATTERN_COMMON),
> [ITEM_IPV6] = TRANS(ITEM_TCP, ITEM_UDP, PATTERN_COMMON),
> [ITEM_TCP] = TRANS(PATTERN_COMMON),
> @@ -72,12 +80,17 @@ static const enum mlx5_nl_flow_trans *const mlx5_nl_flow_trans[] = {
> [ACTION_VOID] = TRANS(BACK),
> [ACTION_PORT_ID] = TRANS(ACTION_VOID, END),
> [ACTION_DROP] = TRANS(ACTION_VOID, END),
> + [ACTION_OF_POP_VLAN] = TRANS(ACTIONS_FATE, ACTIONS_COMMON),
> + [ACTION_OF_PUSH_VLAN] = TRANS(ACTIONS_FATE, ACTIONS_COMMON),
> + [ACTION_OF_SET_VLAN_VID] = TRANS(ACTIONS_FATE, ACTIONS_COMMON),
> + [ACTION_OF_SET_VLAN_PCP] = TRANS(ACTIONS_FATE, ACTIONS_COMMON),
> [END] = NULL,
> };
>
> /** Empty masks for known item types. */
> static const union {
> struct rte_flow_item_eth eth;
> + struct rte_flow_item_vlan vlan;
> struct rte_flow_item_ipv4 ipv4;
> struct rte_flow_item_ipv6 ipv6;
> struct rte_flow_item_tcp tcp;
> @@ -87,6 +100,7 @@ static const union {
> /** Supported masks for known item types. */
> static const struct {
> struct rte_flow_item_eth eth;
> + struct rte_flow_item_vlan vlan;
> struct rte_flow_item_ipv4 ipv4;
> struct rte_flow_item_ipv6 ipv6;
> struct rte_flow_item_tcp tcp;
> @@ -97,6 +111,11 @@ static const struct {
> .dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> .src.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> },
> + .vlan = {
> + /* PCP and VID only, no DEI. */
> + .tci = RTE_BE16(0xefff),
> + .inner_type = RTE_BE16(0xffff),
> + },
> .ipv4.hdr = {
> .next_proto_id = 0xff,
> .src_addr = RTE_BE32(0xffffffff),
> @@ -242,9 +261,13 @@ mlx5_nl_flow_transpose(void *buf,
> unsigned int n;
> uint32_t act_index_cur;
> bool eth_type_set;
> + bool vlan_present;
> + bool vlan_eth_type_set;
> bool ip_proto_set;
> struct nlattr *na_flower;
> struct nlattr *na_flower_act;
> + struct nlattr *na_vlan_id;
> + struct nlattr *na_vlan_priority;
> const enum mlx5_nl_flow_trans *trans;
> const enum mlx5_nl_flow_trans *back;
>
> @@ -256,15 +279,20 @@ mlx5_nl_flow_transpose(void *buf,
> n = 0;
> act_index_cur = 0;
> eth_type_set = false;
> + vlan_present = false;
> + vlan_eth_type_set = false;
> ip_proto_set = false;
> na_flower = NULL;
> na_flower_act = NULL;
> + na_vlan_id = NULL;
> + na_vlan_priority = NULL;
> trans = TRANS(ATTR);
> back = trans;
> trans:
> switch (trans[n++]) {
> union {
> const struct rte_flow_item_eth *eth;
> + const struct rte_flow_item_vlan *vlan;
> const struct rte_flow_item_ipv4 *ipv4;
> const struct rte_flow_item_ipv6 *ipv6;
> const struct rte_flow_item_tcp *tcp;
> @@ -272,6 +300,11 @@ mlx5_nl_flow_transpose(void *buf,
> } spec, mask;
> union {
> const struct rte_flow_action_port_id *port_id;
> + const struct rte_flow_action_of_push_vlan *of_push_vlan;
> + const struct rte_flow_action_of_set_vlan_vid *
> + of_set_vlan_vid;
> + const struct rte_flow_action_of_set_vlan_pcp *
> + of_set_vlan_pcp;
> } conf;
> struct nlmsghdr *nlh;
> struct tcmsg *tcm;
> @@ -408,6 +441,58 @@ mlx5_nl_flow_transpose(void *buf,
> goto error_nobufs;
> ++item;
> break;
> + case ITEM_VLAN:
> + if (item->type != RTE_FLOW_ITEM_TYPE_VLAN)
> + goto trans;
> + mask.vlan = mlx5_nl_flow_item_mask
> + (item, &rte_flow_item_vlan_mask,
> + &mlx5_nl_flow_mask_supported.vlan,
> + &mlx5_nl_flow_mask_empty.vlan,
> + sizeof(mlx5_nl_flow_mask_supported.vlan), error);
> + if (!mask.vlan)
> + return -rte_errno;
> + if (!eth_type_set &&
> + !mnl_attr_put_u16_check(buf, size,
> + TCA_FLOWER_KEY_ETH_TYPE,
> + RTE_BE16(ETH_P_8021Q)))
> + goto error_nobufs;
> + eth_type_set = 1;
> + vlan_present = 1;
> + if (mask.vlan == &mlx5_nl_flow_mask_empty.vlan) {
> + ++item;
> + break;
> + }
> + spec.vlan = item->spec;
> + if ((mask.vlan->tci & RTE_BE16(0xe000) &&
> + (mask.vlan->tci & RTE_BE16(0xe000)) != RTE_BE16(0xe000)) ||
> + (mask.vlan->tci & RTE_BE16(0x0fff) &&
> + (mask.vlan->tci & RTE_BE16(0x0fff)) != RTE_BE16(0x0fff)) ||
> + (mask.vlan->inner_type &&
> + mask.vlan->inner_type != RTE_BE16(0xffff)))
> + return rte_flow_error_set
> + (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ITEM_MASK,
> + mask.vlan,
> + "no support for partial masks on"
> + " \"tci\" (PCP and VID parts) and"
> + " \"inner_type\" fields");
> + if (mask.vlan->inner_type) {
> + if (!mnl_attr_put_u16_check
> + (buf, size, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
> + spec.vlan->inner_type))
> + goto error_nobufs;
> + vlan_eth_type_set = 1;
> + }
> + if ((mask.vlan->tci & RTE_BE16(0xe000) &&
> + !mnl_attr_put_u8_check
> + (buf, size, TCA_FLOWER_KEY_VLAN_PRIO,
> + (rte_be_to_cpu_16(spec.vlan->tci) >> 13) & 0x7)) ||
> + (mask.vlan->tci & RTE_BE16(0x0fff) &&
> + !mnl_attr_put_u16_check
> + (buf, size, TCA_FLOWER_KEY_VLAN_ID,
> + spec.vlan->tci & RTE_BE16(0x0fff))))
> + goto error_nobufs;
> + ++item;
> + break;
> case ITEM_IPV4:
> if (item->type != RTE_FLOW_ITEM_TYPE_IPV4)
> goto trans;
> @@ -418,12 +503,15 @@ mlx5_nl_flow_transpose(void *buf,
> sizeof(mlx5_nl_flow_mask_supported.ipv4), error);
> if (!mask.ipv4)
> return -rte_errno;
> - if (!eth_type_set &&
> + if ((!eth_type_set || !vlan_eth_type_set) &&
> !mnl_attr_put_u16_check(buf, size,
> + vlan_present ?
> + TCA_FLOWER_KEY_VLAN_ETH_TYPE :
> TCA_FLOWER_KEY_ETH_TYPE,
> RTE_BE16(ETH_P_IP)))
> goto error_nobufs;
> eth_type_set = 1;
> + vlan_eth_type_set = 1;
> if (mask.ipv4 == &mlx5_nl_flow_mask_empty.ipv4) {
> ++item;
> break;
> @@ -470,12 +558,15 @@ mlx5_nl_flow_transpose(void *buf,
> sizeof(mlx5_nl_flow_mask_supported.ipv6), error);
> if (!mask.ipv6)
> return -rte_errno;
> - if (!eth_type_set &&
> + if ((!eth_type_set || !vlan_eth_type_set) &&
> !mnl_attr_put_u16_check(buf, size,
> + vlan_present ?
> + TCA_FLOWER_KEY_VLAN_ETH_TYPE :
> TCA_FLOWER_KEY_ETH_TYPE,
> RTE_BE16(ETH_P_IPV6)))
> goto error_nobufs;
> eth_type_set = 1;
> + vlan_eth_type_set = 1;
> if (mask.ipv6 == &mlx5_nl_flow_mask_empty.ipv6) {
> ++item;
> break;
> @@ -681,6 +772,84 @@ mlx5_nl_flow_transpose(void *buf,
> mnl_attr_nest_end(buf, act_index);
> ++action;
> break;
> + case ACTION_OF_POP_VLAN:
> + if (action->type != RTE_FLOW_ACTION_TYPE_OF_POP_VLAN)
> + goto trans;
> + conf.of_push_vlan = NULL;
> + i = TCA_VLAN_ACT_POP;
> + goto action_of_vlan;
> + case ACTION_OF_PUSH_VLAN:
> + if (action->type != RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN)
> + goto trans;
> + conf.of_push_vlan = action->conf;
> + i = TCA_VLAN_ACT_PUSH;
> + goto action_of_vlan;
> + case ACTION_OF_SET_VLAN_VID:
> + if (action->type != RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID)
> + goto trans;
> + conf.of_set_vlan_vid = action->conf;
> + if (na_vlan_id)
> + goto override_na_vlan_id;
> + i = TCA_VLAN_ACT_MODIFY;
> + goto action_of_vlan;
> + case ACTION_OF_SET_VLAN_PCP:
> + if (action->type != RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP)
> + goto trans;
> + conf.of_set_vlan_pcp = action->conf;
> + if (na_vlan_priority)
> + goto override_na_vlan_priority;
> + i = TCA_VLAN_ACT_MODIFY;
> + goto action_of_vlan;
> +action_of_vlan:
> + act_index =
> + mnl_attr_nest_start_check(buf, size, act_index_cur++);
> + if (!act_index ||
> + !mnl_attr_put_strz_check(buf, size, TCA_ACT_KIND, "vlan"))
> + goto error_nobufs;
> + act = mnl_attr_nest_start_check(buf, size, TCA_ACT_OPTIONS);
> + if (!act)
> + goto error_nobufs;
> + if (!mnl_attr_put_check(buf, size, TCA_VLAN_PARMS,
> + sizeof(struct tc_vlan),
> + &(struct tc_vlan){
> + .action = TC_ACT_PIPE,
> + .v_action = i,
> + }))
> + goto error_nobufs;
> + if (i == TCA_VLAN_ACT_POP) {
> + mnl_attr_nest_end(buf, act);
> + ++action;
> + break;
> + }
> + if (i == TCA_VLAN_ACT_PUSH &&
> + !mnl_attr_put_u16_check(buf, size,
> + TCA_VLAN_PUSH_VLAN_PROTOCOL,
> + conf.of_push_vlan->ethertype))
> + goto error_nobufs;
> + na_vlan_id = mnl_nlmsg_get_payload_tail(buf);
> + if (!mnl_attr_put_u16_check(buf, size, TCA_VLAN_PAD, 0))
> + goto error_nobufs;
> + na_vlan_priority = mnl_nlmsg_get_payload_tail(buf);
> + if (!mnl_attr_put_u8_check(buf, size, TCA_VLAN_PAD, 0))
> + goto error_nobufs;
> + mnl_attr_nest_end(buf, act);
> + mnl_attr_nest_end(buf, act_index);
> + if (action->type == RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID) {
> +override_na_vlan_id:
> + na_vlan_id->nla_type = TCA_VLAN_PUSH_VLAN_ID;
> + *(uint16_t *)mnl_attr_get_payload(na_vlan_id) =
> + rte_be_to_cpu_16
> + (conf.of_set_vlan_vid->vlan_vid);
> + } else if (action->type ==
> + RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP) {
> +override_na_vlan_priority:
> + na_vlan_priority->nla_type =
> + TCA_VLAN_PUSH_VLAN_PRIORITY;
> + *(uint8_t *)mnl_attr_get_payload(na_vlan_priority) =
> + conf.of_set_vlan_pcp->vlan_pcp;
> + }
> + ++action;
> + break;
I'm wondering if there's no need to check the existence of VLAN in pattern when
having VLAN modification actions. For example, if flow has POP_VLAN action, its
pattern has to have VLAN item, doesn't it? Even though kernel driver has such
validation checks, mlx5_flow_validate() can't validate it.
In the PRM,
8.18.2.7 Packet Classification Ambiguities
...
In addition, a flow should not match or attempt to modify (Modify Header
action, Pop VLAN action) non-existing fields of a packet, as defined by
the packet classification process.
...
Thanks,
Yongseok
> case END:
> if (item->type != RTE_FLOW_ITEM_TYPE_END ||
> action->type != RTE_FLOW_ACTION_TYPE_END)
> --
> 2.11.0
next prev parent reply other threads:[~2018-07-12 1:10 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-27 18:08 [dpdk-dev] [PATCH 0/6] net/mlx5: add support for " Adrien Mazarguil
2018-06-27 18:08 ` [dpdk-dev] [PATCH 1/6] net/mlx5: lay groundwork for switch offloads Adrien Mazarguil
2018-07-12 0:17 ` Yongseok Koh
2018-07-12 10:46 ` Adrien Mazarguil
2018-07-12 17:33 ` Yongseok Koh
2018-06-27 18:08 ` [dpdk-dev] [PATCH 2/6] net/mlx5: add framework for switch flow rules Adrien Mazarguil
2018-07-12 0:59 ` Yongseok Koh
2018-07-12 10:46 ` Adrien Mazarguil
2018-07-12 18:25 ` Yongseok Koh
2018-06-27 18:08 ` [dpdk-dev] [PATCH 3/6] net/mlx5: add fate actions to " Adrien Mazarguil
2018-07-12 1:00 ` Yongseok Koh
2018-06-27 18:08 ` [dpdk-dev] [PATCH 4/6] net/mlx5: add L2-L4 pattern items " Adrien Mazarguil
2018-07-12 1:02 ` Yongseok Koh
2018-06-27 18:08 ` [dpdk-dev] [PATCH 5/6] net/mlx5: add VLAN item and actions " Adrien Mazarguil
2018-07-12 1:10 ` Yongseok Koh [this message]
2018-07-12 10:47 ` Adrien Mazarguil
2018-07-12 18:49 ` Yongseok Koh
2018-06-27 18:08 ` [dpdk-dev] [PATCH 6/6] net/mlx5: add port ID pattern item " Adrien Mazarguil
2018-07-12 1:13 ` Yongseok Koh
2018-06-28 9:05 ` [dpdk-dev] [PATCH 0/6] net/mlx5: add support for " Nélio Laranjeiro
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 " Adrien Mazarguil
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 1/6] net/mlx5: lay groundwork for switch offloads Adrien Mazarguil
2018-07-14 1:29 ` Yongseok Koh
2018-07-23 21:40 ` Ferruh Yigit
2018-07-24 0:50 ` Stephen Hemminger
2018-07-24 4:35 ` Shahaf Shuler
2018-07-24 19:33 ` Stephen Hemminger
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 2/6] net/mlx5: add framework for switch flow rules Adrien Mazarguil
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 3/6] net/mlx5: add fate actions to " Adrien Mazarguil
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 4/6] net/mlx5: add L2-L4 pattern items " Adrien Mazarguil
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 5/6] net/mlx5: add VLAN item and actions " Adrien Mazarguil
2018-07-13 9:40 ` [dpdk-dev] [PATCH v2 6/6] net/mlx5: add port ID pattern item " Adrien Mazarguil
2018-07-22 11:21 ` [dpdk-dev] [PATCH v2 0/6] net/mlx5: add support for " Shahaf Shuler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180712011024.GG69686@yongseok-MBP.local \
--to=yskoh@mellanox.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=nelio.laranjeiro@6wind.com \
--cc=shahafs@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).