diff mbox series

[net-next] sfc: support offloading TC VLAN push/pop actions to the MAE

Message ID 20230216160442.48394-1-edward.cree@amd.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] sfc: support offloading TC VLAN push/pop actions to the MAE | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch warning WARNING: line length of 112 exceeds 80 columns WARNING: line length of 114 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns WARNING: line length of 94 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 1 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

edward.cree@amd.com Feb. 16, 2023, 4:04 p.m. UTC 
From: Edward Cree <ecree.xilinx@gmail.com>

EF100 can pop and/or push up to two VLAN tags.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/mae.c  | 43 ++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/mcdi.h |  5 ++++
 drivers/net/ethernet/sfc/tc.c   | 53 +++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc.h   |  4 +++
 4 files changed, 105 insertions(+)

Comments

Martin Habets Feb. 17, 2023, 9 a.m. UTC | #1 
On Thu, Feb 16, 2023 at 04:04:42PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> EF100 can pop and/or push up to two VLAN tags.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  drivers/net/ethernet/sfc/mae.c  | 43 ++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/mcdi.h |  5 ++++
>  drivers/net/ethernet/sfc/tc.c   | 53 +++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/tc.h   |  4 +++
>  4 files changed, 105 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
> index 6321fd393fc3..7ae5b22af624 100644
> --- a/drivers/net/ethernet/sfc/mae.c
> +++ b/drivers/net/ethernet/sfc/mae.c
> @@ -679,9 +679,40 @@ int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
>  {
>  	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_ACTION_SET_ALLOC_OUT_LEN);
>  	MCDI_DECLARE_BUF(inbuf, MC_CMD_MAE_ACTION_SET_ALLOC_IN_LEN);
> +	unsigned char vlan_push, vlan_pop;
>  	size_t outlen;
>  	int rc;
>  
> +	/* Translate vlan actions from bitmask to count */
> +	switch (act->vlan_push) {
> +	case 0:
> +	case 1:
> +		vlan_push = act->vlan_push;
> +		break;
> +	case 2: /* can't happen */

Use fallthrough here.

> +	default:
> +		return -EINVAL;
> +	case 3:
> +		vlan_push = 2;
> +		break;
> +	}
> +	switch (act->vlan_pop) {
> +	case 0:
> +	case 1:
> +		vlan_pop = act->vlan_pop;
> +		break;
> +	case 2: /* can't happen */

and here.

Martin

> +	default:
> +		return -EINVAL;
> +	case 3:
> +		vlan_pop = 2;
> +		break;
> +	}
> +
> +	MCDI_POPULATE_DWORD_2(inbuf, MAE_ACTION_SET_ALLOC_IN_FLAGS,
> +			      MAE_ACTION_SET_ALLOC_IN_VLAN_PUSH, vlan_push,
> +			      MAE_ACTION_SET_ALLOC_IN_VLAN_POP, vlan_pop);
> +
>  	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_SRC_MAC_ID,
>  		       MC_CMD_MAE_MAC_ADDR_ALLOC_OUT_MAC_ID_NULL);
>  	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_DST_MAC_ID,
> @@ -694,6 +725,18 @@ int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
>  			       MC_CMD_MAE_COUNTER_ALLOC_OUT_COUNTER_ID_NULL);
>  	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_COUNTER_LIST_ID,
>  		       MC_CMD_MAE_COUNTER_LIST_ALLOC_OUT_COUNTER_LIST_ID_NULL);
> +	if (act->vlan_push & 1) {
> +		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN0_TCI_BE,
> +				 act->vlan_tci[0]);
> +		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN0_PROTO_BE,
> +				 act->vlan_proto[0]);
> +	}
> +	if (act->vlan_push & 2) {
> +		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN1_TCI_BE,
> +				 act->vlan_tci[1]);
> +		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN1_PROTO_BE,
> +				 act->vlan_proto[1]);
> +	}
>  	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_ENCAP_HEADER_ID,
>  		       MC_CMD_MAE_ENCAP_HEADER_ALLOC_OUT_ENCAP_HEADER_ID_NULL);
>  	if (act->deliver)
> diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h
> index b139b76febff..454e9d51a4c2 100644
> --- a/drivers/net/ethernet/sfc/mcdi.h
> +++ b/drivers/net/ethernet/sfc/mcdi.h
> @@ -233,6 +233,11 @@ void efx_mcdi_sensor_event(struct efx_nic *efx, efx_qword_t *ev);
>  	((void)BUILD_BUG_ON_ZERO(_field ## _LEN != 2),  \
>  	le16_to_cpu(*(__force const __le16 *)MCDI_STRUCT_PTR(_buf, _field)))
>  /* Write a 16-bit field defined in the protocol as being big-endian. */
> +#define MCDI_SET_WORD_BE(_buf, _field, _value) do {			\
> +	BUILD_BUG_ON(MC_CMD_ ## _field ## _LEN != 2);			\
> +	BUILD_BUG_ON(MC_CMD_ ## _field ## _OFST & 1);			\
> +	*(__force __be16 *)MCDI_PTR(_buf, _field) = (_value);		\
> +	} while (0)
>  #define MCDI_STRUCT_SET_WORD_BE(_buf, _field, _value) do {		\
>  	BUILD_BUG_ON(_field ## _LEN != 2);				\
>  	BUILD_BUG_ON(_field ## _OFST & 1);				\
> diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
> index deeaab9ee761..195c288736be 100644
> --- a/drivers/net/ethernet/sfc/tc.c
> +++ b/drivers/net/ethernet/sfc/tc.c
> @@ -286,6 +286,10 @@ static int efx_tc_flower_parse_match(struct efx_nic *efx,
>  
>  /* For details of action order constraints refer to SF-123102-TC-1§12.6.1 */
>  enum efx_tc_action_order {
> +	EFX_TC_AO_VLAN1_POP,
> +	EFX_TC_AO_VLAN0_POP,
> +	EFX_TC_AO_VLAN0_PUSH,
> +	EFX_TC_AO_VLAN1_PUSH,
>  	EFX_TC_AO_COUNT,
>  	EFX_TC_AO_DELIVER
>  };
> @@ -294,6 +298,22 @@ static bool efx_tc_flower_action_order_ok(const struct efx_tc_action_set *act,
>  					  enum efx_tc_action_order new)
>  {
>  	switch (new) {
> +	case EFX_TC_AO_VLAN0_POP:
> +		if (act->vlan_pop & 1)
> +			return false;
> +		fallthrough;
> +	case EFX_TC_AO_VLAN1_POP:
> +		if (act->vlan_pop & 2)
> +			return false;
> +		fallthrough;
> +	case EFX_TC_AO_VLAN0_PUSH:
> +		if (act->vlan_push & 1)
> +			return false;
> +		fallthrough;
> +	case EFX_TC_AO_VLAN1_PUSH:
> +		if (act->vlan_push & 2)
> +			return false;
> +		fallthrough;
>  	case EFX_TC_AO_COUNT:
>  		if (act->count)
>  			return false;
> @@ -393,6 +413,8 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
>  
>  	flow_action_for_each(i, fa, &fr->action) {
>  		struct efx_tc_action_set save;
> +		int depth;
> +		u16 tci;
>  
>  		if (!act) {
>  			/* more actions after a non-pipe action */
> @@ -494,6 +516,37 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
>  			}
>  			*act = save;
>  			break;
> +		case FLOW_ACTION_VLAN_POP:
> +			if (act->vlan_push & 2) {
> +				act->vlan_push &= ~2;
> +			} else if (act->vlan_push & 1) {
> +				act->vlan_push &= ~1;
> +			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN0_POP)) {
> +				act->vlan_pop |= 1;
> +			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN1_POP)) {
> +				act->vlan_pop |= 2;
> +			} else {
> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pops, or action order violated");
> +				rc = -EINVAL;
> +				goto release;
> +			}
> +			break;
> +		case FLOW_ACTION_VLAN_PUSH:
> +			if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN0_PUSH)) {
> +				depth = 0;
> +			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN1_PUSH)) {
> +				depth = 1;
> +			} else {
> +				rc = -EINVAL;
> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pushes, or action order violated");
> +				goto release;
> +			}
> +			tci = fa->vlan.vid & 0x0fff;
> +			tci |= fa->vlan.prio << 13;
> +			act->vlan_push |= (1 << depth);
> +			act->vlan_tci[depth] = cpu_to_be16(tci);
> +			act->vlan_proto[depth] = fa->vlan.proto;
> +			break;
>  		default:
>  			NL_SET_ERR_MSG_FMT_MOD(extack, "Unhandled action %u",
>  					       fa->id);
> diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
> index 418ce8c13a06..542853f60c2a 100644
> --- a/drivers/net/ethernet/sfc/tc.h
> +++ b/drivers/net/ethernet/sfc/tc.h
> @@ -19,7 +19,11 @@
>  #define IS_ALL_ONES(v)	(!(typeof (v))~(v))
>  
>  struct efx_tc_action_set {
> +	u16 vlan_push:2;
> +	u16 vlan_pop:2;
>  	u16 deliver:1;
> +	__be16 vlan_tci[2]; /* TCIs for vlan_push */
> +	__be16 vlan_proto[2]; /* Ethertypes for vlan_push */
>  	struct efx_tc_counter_index *count;
>  	u32 dest_mport;
>  	u32 fw_id; /* index of this entry in firmware actions table */
Leon Romanovsky Feb. 19, 2023, 9:21 a.m. UTC | #2 
On Thu, Feb 16, 2023 at 04:04:42PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> EF100 can pop and/or push up to two VLAN tags.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  drivers/net/ethernet/sfc/mae.c  | 43 ++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/mcdi.h |  5 ++++
>  drivers/net/ethernet/sfc/tc.c   | 53 +++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/tc.h   |  4 +++
>  4 files changed, 105 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
> index 6321fd393fc3..7ae5b22af624 100644
> --- a/drivers/net/ethernet/sfc/mae.c
> +++ b/drivers/net/ethernet/sfc/mae.c
> @@ -679,9 +679,40 @@ int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
>  {
>  	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_ACTION_SET_ALLOC_OUT_LEN);
>  	MCDI_DECLARE_BUF(inbuf, MC_CMD_MAE_ACTION_SET_ALLOC_IN_LEN);
> +	unsigned char vlan_push, vlan_pop;
>  	size_t outlen;
>  	int rc;
>  
> +	/* Translate vlan actions from bitmask to count */
> +	switch (act->vlan_push) {
> +	case 0:
> +	case 1:
> +		vlan_push = act->vlan_push;
> +		break;
> +	case 2: /* can't happen */

There is no need in case here as "default" will catch.

> +	default:
> +		return -EINVAL;
> +	case 3:
> +		vlan_push = 2;
> +		break;
> +	}
> +	switch (act->vlan_pop) {
> +	case 0:
> +	case 1:
> +		vlan_pop = act->vlan_pop;
> +		break;
> +	case 2: /* can't happen */
> +	default:
> +		return -EINVAL;

Please rely switch-case semantics and don't put default in the middle.


> +	case 3:
> +		vlan_pop = 2;
> +		break;
> +	}

Thanks
Edward Cree Feb. 21, 2023, 8:32 p.m. UTC | #3 
On 19/02/2023 09:21, Leon Romanovsky wrote:
> On Thu, Feb 16, 2023 at 04:04:42PM +0000, edward.cree@amd.com wrote:
>> From: Edward Cree <ecree.xilinx@gmail.com>
>>
>> EF100 can pop and/or push up to two VLAN tags.
>>
>> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
...
>> +	/* Translate vlan actions from bitmask to count */
>> +	switch (act->vlan_push) {
>> +	case 0:
>> +	case 1:
>> +		vlan_push = act->vlan_push;
>> +		break;
>> +	case 2: /* can't happen */
> 
> There is no need in case here as "default" will catch.
> 
>> +	default:
>> +		return -EINVAL;
>> +	case 3:
>> +		vlan_push = 2;
>> +		break;
>> +	}
>> +	switch (act->vlan_pop) {
>> +	case 0:
>> +	case 1:
>> +		vlan_pop = act->vlan_pop;
>> +		break;
>> +	case 2: /* can't happen */
>> +	default:
>> +		return -EINVAL;
> 
> Please rely switch-case semantics and don't put default in the middle.

It's legal C and as far as I can tell there's nothing in coding-style.rst
 about it; I did it this way so as to put the cases in the logical(?)
 ascending order and try to make the code self-document the possible
 values of the act-> fields.
Arguably it's the 'default:' rather than the 'case 2:' that's unnecessary
 as the switch argument is an unsigned:2 bitfield, so it can only take on
 these four values.
Although on revisiting this code I wonder if it makes more sense just to
 use the 'count' (rather than 'bitmask') form throughout, including in
 act->vlan_push/pop; it makes the tc.c side of the code slightly more
 involved, but gets rid of this translation entirely.  WDYT?

-ed
Martin Habets Feb. 22, 2023, 8:56 a.m. UTC | #4 
On Tue, Feb 21, 2023 at 08:32:13PM +0000, Edward Cree wrote:
> On 19/02/2023 09:21, Leon Romanovsky wrote:
> > On Thu, Feb 16, 2023 at 04:04:42PM +0000, edward.cree@amd.com wrote:
> >> From: Edward Cree <ecree.xilinx@gmail.com>
> >>
> >> EF100 can pop and/or push up to two VLAN tags.
> >>
> >> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ...
> >> +	/* Translate vlan actions from bitmask to count */
> >> +	switch (act->vlan_push) {
> >> +	case 0:
> >> +	case 1:
> >> +		vlan_push = act->vlan_push;
> >> +		break;
> >> +	case 2: /* can't happen */
> > 
> > There is no need in case here as "default" will catch.
> > 
> >> +	default:
> >> +		return -EINVAL;
> >> +	case 3:
> >> +		vlan_push = 2;
> >> +		break;
> >> +	}
> >> +	switch (act->vlan_pop) {
> >> +	case 0:
> >> +	case 1:
> >> +		vlan_pop = act->vlan_pop;
> >> +		break;
> >> +	case 2: /* can't happen */
> >> +	default:
> >> +		return -EINVAL;
> > 
> > Please rely switch-case semantics and don't put default in the middle.
> 
> It's legal C and as far as I can tell there's nothing in coding-style.rst
>  about it; I did it this way so as to put the cases in the logical(?)
>  ascending order and try to make the code self-document the possible
>  values of the act-> fields.
> Arguably it's the 'default:' rather than the 'case 2:' that's unnecessary
>  as the switch argument is an unsigned:2 bitfield, so it can only take on
>  these four values.

Can you replace the switch statement with
 vlan_push = act->vlan_push & 1 + act->vlan_push & 2;
Even then it would seem prudent to guard against  act->vlan_push == 2.

Martin

> Although on revisiting this code I wonder if it makes more sense just to
>  use the 'count' (rather than 'bitmask') form throughout, including in
>  act->vlan_push/pop; it makes the tc.c side of the code slightly more
>  involved, but gets rid of this translation entirely.  WDYT?
> 
> -ed
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index 6321fd393fc3..7ae5b22af624 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -679,9 +679,40 @@  int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
 {
 	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_ACTION_SET_ALLOC_OUT_LEN);
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_MAE_ACTION_SET_ALLOC_IN_LEN);
+	unsigned char vlan_push, vlan_pop;
 	size_t outlen;
 	int rc;
 
+	/* Translate vlan actions from bitmask to count */
+	switch (act->vlan_push) {
+	case 0:
+	case 1:
+		vlan_push = act->vlan_push;
+		break;
+	case 2: /* can't happen */
+	default:
+		return -EINVAL;
+	case 3:
+		vlan_push = 2;
+		break;
+	}
+	switch (act->vlan_pop) {
+	case 0:
+	case 1:
+		vlan_pop = act->vlan_pop;
+		break;
+	case 2: /* can't happen */
+	default:
+		return -EINVAL;
+	case 3:
+		vlan_pop = 2;
+		break;
+	}
+
+	MCDI_POPULATE_DWORD_2(inbuf, MAE_ACTION_SET_ALLOC_IN_FLAGS,
+			      MAE_ACTION_SET_ALLOC_IN_VLAN_PUSH, vlan_push,
+			      MAE_ACTION_SET_ALLOC_IN_VLAN_POP, vlan_pop);
+
 	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_SRC_MAC_ID,
 		       MC_CMD_MAE_MAC_ADDR_ALLOC_OUT_MAC_ID_NULL);
 	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_DST_MAC_ID,
@@ -694,6 +725,18 @@  int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
 			       MC_CMD_MAE_COUNTER_ALLOC_OUT_COUNTER_ID_NULL);
 	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_COUNTER_LIST_ID,
 		       MC_CMD_MAE_COUNTER_LIST_ALLOC_OUT_COUNTER_LIST_ID_NULL);
+	if (act->vlan_push & 1) {
+		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN0_TCI_BE,
+				 act->vlan_tci[0]);
+		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN0_PROTO_BE,
+				 act->vlan_proto[0]);
+	}
+	if (act->vlan_push & 2) {
+		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN1_TCI_BE,
+				 act->vlan_tci[1]);
+		MCDI_SET_WORD_BE(inbuf, MAE_ACTION_SET_ALLOC_IN_VLAN1_PROTO_BE,
+				 act->vlan_proto[1]);
+	}
 	MCDI_SET_DWORD(inbuf, MAE_ACTION_SET_ALLOC_IN_ENCAP_HEADER_ID,
 		       MC_CMD_MAE_ENCAP_HEADER_ALLOC_OUT_ENCAP_HEADER_ID_NULL);
 	if (act->deliver)
diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h
index b139b76febff..454e9d51a4c2 100644
--- a/drivers/net/ethernet/sfc/mcdi.h
+++ b/drivers/net/ethernet/sfc/mcdi.h
@@ -233,6 +233,11 @@  void efx_mcdi_sensor_event(struct efx_nic *efx, efx_qword_t *ev);
 	((void)BUILD_BUG_ON_ZERO(_field ## _LEN != 2),  \
 	le16_to_cpu(*(__force const __le16 *)MCDI_STRUCT_PTR(_buf, _field)))
 /* Write a 16-bit field defined in the protocol as being big-endian. */
+#define MCDI_SET_WORD_BE(_buf, _field, _value) do {			\
+	BUILD_BUG_ON(MC_CMD_ ## _field ## _LEN != 2);			\
+	BUILD_BUG_ON(MC_CMD_ ## _field ## _OFST & 1);			\
+	*(__force __be16 *)MCDI_PTR(_buf, _field) = (_value);		\
+	} while (0)
 #define MCDI_STRUCT_SET_WORD_BE(_buf, _field, _value) do {		\
 	BUILD_BUG_ON(_field ## _LEN != 2);				\
 	BUILD_BUG_ON(_field ## _OFST & 1);				\
diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index deeaab9ee761..195c288736be 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -286,6 +286,10 @@  static int efx_tc_flower_parse_match(struct efx_nic *efx,
 
 /* For details of action order constraints refer to SF-123102-TC-1§12.6.1 */
 enum efx_tc_action_order {
+	EFX_TC_AO_VLAN1_POP,
+	EFX_TC_AO_VLAN0_POP,
+	EFX_TC_AO_VLAN0_PUSH,
+	EFX_TC_AO_VLAN1_PUSH,
 	EFX_TC_AO_COUNT,
 	EFX_TC_AO_DELIVER
 };
@@ -294,6 +298,22 @@  static bool efx_tc_flower_action_order_ok(const struct efx_tc_action_set *act,
 					  enum efx_tc_action_order new)
 {
 	switch (new) {
+	case EFX_TC_AO_VLAN0_POP:
+		if (act->vlan_pop & 1)
+			return false;
+		fallthrough;
+	case EFX_TC_AO_VLAN1_POP:
+		if (act->vlan_pop & 2)
+			return false;
+		fallthrough;
+	case EFX_TC_AO_VLAN0_PUSH:
+		if (act->vlan_push & 1)
+			return false;
+		fallthrough;
+	case EFX_TC_AO_VLAN1_PUSH:
+		if (act->vlan_push & 2)
+			return false;
+		fallthrough;
 	case EFX_TC_AO_COUNT:
 		if (act->count)
 			return false;
@@ -393,6 +413,8 @@  static int efx_tc_flower_replace(struct efx_nic *efx,
 
 	flow_action_for_each(i, fa, &fr->action) {
 		struct efx_tc_action_set save;
+		int depth;
+		u16 tci;
 
 		if (!act) {
 			/* more actions after a non-pipe action */
@@ -494,6 +516,37 @@  static int efx_tc_flower_replace(struct efx_nic *efx,
 			}
 			*act = save;
 			break;
+		case FLOW_ACTION_VLAN_POP:
+			if (act->vlan_push & 2) {
+				act->vlan_push &= ~2;
+			} else if (act->vlan_push & 1) {
+				act->vlan_push &= ~1;
+			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN0_POP)) {
+				act->vlan_pop |= 1;
+			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN1_POP)) {
+				act->vlan_pop |= 2;
+			} else {
+				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pops, or action order violated");
+				rc = -EINVAL;
+				goto release;
+			}
+			break;
+		case FLOW_ACTION_VLAN_PUSH:
+			if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN0_PUSH)) {
+				depth = 0;
+			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN1_PUSH)) {
+				depth = 1;
+			} else {
+				rc = -EINVAL;
+				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pushes, or action order violated");
+				goto release;
+			}
+			tci = fa->vlan.vid & 0x0fff;
+			tci |= fa->vlan.prio << 13;
+			act->vlan_push |= (1 << depth);
+			act->vlan_tci[depth] = cpu_to_be16(tci);
+			act->vlan_proto[depth] = fa->vlan.proto;
+			break;
 		default:
 			NL_SET_ERR_MSG_FMT_MOD(extack, "Unhandled action %u",
 					       fa->id);
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index 418ce8c13a06..542853f60c2a 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -19,7 +19,11 @@ 
 #define IS_ALL_ONES(v)	(!(typeof (v))~(v))
 
 struct efx_tc_action_set {
+	u16 vlan_push:2;
+	u16 vlan_pop:2;
 	u16 deliver:1;
+	__be16 vlan_tci[2]; /* TCIs for vlan_push */
+	__be16 vlan_proto[2]; /* Ethertypes for vlan_push */
 	struct efx_tc_counter_index *count;
 	u32 dest_mport;
 	u32 fw_id; /* index of this entry in firmware actions table */