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.mazarg...@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