netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE
@ 2023-02-23 23:50 edward.cree
  2023-02-24 10:07 ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: edward.cree @ 2023-02-23 23:50 UTC (permalink / raw)
  To: netdev; +Cc: Edward Cree, linux-net-drivers, habetsm.xilinx, leon

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>
---
Changed in v2: reworked act->vlan_push/pop to be counts rather than bitmasks,
 and simplified the corresponding efx_tc_action_order handling.

 drivers/net/ethernet/sfc/mae.c  | 16 +++++++++++++
 drivers/net/ethernet/sfc/mcdi.h |  5 +++++
 drivers/net/ethernet/sfc/tc.c   | 40 +++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc.h   |  4 ++++
 4 files changed, 65 insertions(+)

diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index 6321fd393fc3..142b3d6ae6aa 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -682,6 +682,10 @@ int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act)
 	size_t outlen;
 	int rc;
 
+	MCDI_POPULATE_DWORD_2(inbuf, MAE_ACTION_SET_ALLOC_IN_FLAGS,
+			      MAE_ACTION_SET_ALLOC_IN_VLAN_PUSH, act->vlan_push,
+			      MAE_ACTION_SET_ALLOC_IN_VLAN_POP, act->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 +698,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) {
+		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..12b34320bc81 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -286,6 +286,8 @@ 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_VLAN_POP,
+	EFX_TC_AO_VLAN_PUSH,
 	EFX_TC_AO_COUNT,
 	EFX_TC_AO_DELIVER
 };
@@ -294,6 +296,20 @@ 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_VLAN_POP:
+		if (act->vlan_pop >= 2)
+			return false;
+		/* If we've already pushed a VLAN, we can't then pop it;
+		 * the hardware would instead try to pop an existing VLAN
+		 * before pushing the new one.
+		 */
+		if (act->vlan_push)
+			return false;
+		fallthrough;
+	case EFX_TC_AO_VLAN_PUSH:
+		if (act->vlan_push >= 2)
+			return false;
+		fallthrough;
 	case EFX_TC_AO_COUNT:
 		if (act->count)
 			return false;
@@ -393,6 +409,7 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
 
 	flow_action_for_each(i, fa, &fr->action) {
 		struct efx_tc_action_set save;
+		u16 tci;
 
 		if (!act) {
 			/* more actions after a non-pipe action */
@@ -494,6 +511,29 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
 			}
 			*act = save;
 			break;
+		case FLOW_ACTION_VLAN_POP:
+			if (act->vlan_push) {
+				act->vlan_push--;
+			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN_POP)) {
+				act->vlan_pop++;
+			} 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_VLAN_PUSH)) {
+				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_tci[act->vlan_push] = cpu_to_be16(tci);
+			act->vlan_proto[act->vlan_push] = fa->vlan.proto;
+			act->vlan_push++;
+			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 */

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE
  2023-02-23 23:50 [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE edward.cree
@ 2023-02-24 10:07 ` Simon Horman
  2023-02-27 21:33   ` Edward Cree
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2023-02-24 10:07 UTC (permalink / raw)
  To: edward.cree; +Cc: netdev, Edward Cree, linux-net-drivers, habetsm.xilinx, leon

On Thu, Feb 23, 2023 at 11:50:26PM +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>
> ---
> Changed in v2: reworked act->vlan_push/pop to be counts rather than bitmasks,
>  and simplified the corresponding efx_tc_action_order handling.

This looks good to me.

As you'll need to repost as a non-RFC I've added a few nits inline.
But those notwithstanding,

Reviewed-by: Simon Horman <simon.horman@corigine.com>

...

> diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
> index deeaab9ee761..12b34320bc81 100644
> --- a/drivers/net/ethernet/sfc/tc.c
> +++ b/drivers/net/ethernet/sfc/tc.c

...

> @@ -494,6 +511,29 @@ static int efx_tc_flower_replace(struct efx_nic *efx,
>  			}
>  			*act = save;
>  			break;
> +		case FLOW_ACTION_VLAN_POP:
> +			if (act->vlan_push) {
> +				act->vlan_push--;
> +			} else if (efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN_POP)) {
> +				act->vlan_pop++;
> +			} else {
> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pops, or action order violated");

nit: I'm not sure if there is anything to be done about it,
     but checkpatch complains about ling lines here...

> +				rc = -EINVAL;
> +				goto release;
> +			}
> +			break;
> +		case FLOW_ACTION_VLAN_PUSH:
> +			if (!efx_tc_flower_action_order_ok(act, EFX_TC_AO_VLAN_PUSH)) {
> +				rc = -EINVAL;
> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pushes, or action order violated");

... and here.

> +				goto release;
> +			}
> +			tci = fa->vlan.vid & 0x0fff;
> +			tci |= fa->vlan.prio << 13;

nit: Maybe VLAN_PRIO_SHIFT and VLAN_VID_MASK can be used here.

> +			act->vlan_tci[act->vlan_push] = cpu_to_be16(tci);
> +			act->vlan_proto[act->vlan_push] = fa->vlan.proto;
> +			act->vlan_push++;
> +			break;
>  		default:
>  			NL_SET_ERR_MSG_FMT_MOD(extack, "Unhandled action %u",
>  					       fa->id);

...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE
  2023-02-24 10:07 ` Simon Horman
@ 2023-02-27 21:33   ` Edward Cree
  2023-02-28  9:01     ` Martin Habets
  0 siblings, 1 reply; 4+ messages in thread
From: Edward Cree @ 2023-02-27 21:33 UTC (permalink / raw)
  To: Simon Horman, edward.cree; +Cc: netdev, linux-net-drivers, habetsm.xilinx, leon

On 24/02/2023 10:07, Simon Horman wrote:
> On Thu, Feb 23, 2023 at 11:50:26PM +0000, edward.cree@amd.com wrote:
>> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pops, or action order violated");
> 
> nit: I'm not sure if there is anything to be done about it,
>      but checkpatch complains about ling lines here...

Yeah I don't think these can be helped.  Breaking up the
 containing function (to reduce indent depth) would be
 rather synthetic imho, most of it wouldn't even be able
 to be shared with the decap and conntrack versions when
 those get added.)

>> +			}
>> +			tci = fa->vlan.vid & 0x0fff;
>> +			tci |= fa->vlan.prio << 13;
> 
> nit: Maybe VLAN_PRIO_SHIFT and VLAN_VID_MASK can be used here.

Yep good suggestion, incorporated for v3.
Thanks for the review.

-ed

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE
  2023-02-27 21:33   ` Edward Cree
@ 2023-02-28  9:01     ` Martin Habets
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Habets @ 2023-02-28  9:01 UTC (permalink / raw)
  To: Edward Cree; +Cc: Simon Horman, edward.cree, netdev, linux-net-drivers, leon

On Mon, Feb 27, 2023 at 09:33:49PM +0000, Edward Cree wrote:
> On 24/02/2023 10:07, Simon Horman wrote:
> > On Thu, Feb 23, 2023 at 11:50:26PM +0000, edward.cree@amd.com wrote:
> >> +				NL_SET_ERR_MSG_MOD(extack, "More than two VLAN pops, or action order violated");
> > 
> > nit: I'm not sure if there is anything to be done about it,
> >      but checkpatch complains about ling lines here...
> 
> Yeah I don't think these can be helped.  Breaking up the
>  containing function (to reduce indent depth) would be
>  rather synthetic imho, most of it wouldn't even be able
>  to be shared with the decap and conntrack versions when
>  those get added.)

You can put the string on it's own line, i.e. align it under
extack. I think that will pacify checkpatch.

Martin

> 
> >> +			}
> >> +			tci = fa->vlan.vid & 0x0fff;
> >> +			tci |= fa->vlan.prio << 13;
> > 
> > nit: Maybe VLAN_PRIO_SHIFT and VLAN_VID_MASK can be used here.
> 
> Yep good suggestion, incorporated for v3.
> Thanks for the review.
> 
> -ed

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-28  9:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-23 23:50 [RFC PATCH v2 net-next] sfc: support offloading TC VLAN push/pop actions to the MAE edward.cree
2023-02-24 10:07 ` Simon Horman
2023-02-27 21:33   ` Edward Cree
2023-02-28  9:01     ` Martin Habets

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).