[PATCH RFC v1 net-next] net: ethernet: mtk_ppe_offload: Allow QinQ

[PATCH RFC v1 net-next] net: ethernet: mtk_ppe_offload: Allow QinQ

[Eric Woudstra]

mtk_foe_entry_set_vlan() in mtk_ppe.c already seems to support
double vlan tagging, but mtk_flow_offload_replace() in
mtk_ppe_offload.c only allows for 1 vlan tag, optionally in
combination with pppoe and dsa tags.

This patch adds QinQ support to mtk_flow_offload_replace().

Only PPPoE-in-Q (as before) and Q-in-Q are allowed. A combination
of PPPoE and Q-in-Q is not allowed.

As I do not have any documentation of the ppe hardware, I do not
know if there is any other reason to not implement Q-in-Q in
mtk_flow_offload_replace().

Tested on the BPI-R3(mini), on non-dsa-ports and dsa-ports.

Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
---
 .../net/ethernet/mediatek/mtk_ppe_offload.c   | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
index f20bb390df3a..c19789883a9d 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
@@ -34,8 +34,10 @@ struct mtk_flow_data {
 	u16 vlan_in;
 
 	struct {
-		u16 id;
-		__be16 proto;
+		struct {
+			u16 id;
+			__be16 proto;
+		} vlans[2];
 		u8 num;
 	} vlan;
 	struct {
@@ -349,18 +351,19 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
 		case FLOW_ACTION_CSUM:
 			break;
 		case FLOW_ACTION_VLAN_PUSH:
-			if (data.vlan.num == 1 ||
+			if (data.vlan.num + data.pppoe.num == 2 ||
 			    act->vlan.proto != htons(ETH_P_8021Q))
 				return -EOPNOTSUPP;
 
-			data.vlan.id = act->vlan.vid;
-			data.vlan.proto = act->vlan.proto;
+			data.vlan.vlans[data.vlan.num].id = act->vlan.vid;
+			data.vlan.vlans[data.vlan.num].proto = act->vlan.proto;
 			data.vlan.num++;
 			break;
 		case FLOW_ACTION_VLAN_POP:
 			break;
 		case FLOW_ACTION_PPPOE_PUSH:
-			if (data.pppoe.num == 1)
+			if (data.pppoe.num == 1 ||
+			    data.vlan.num == 2)
 				return -EOPNOTSUPP;
 
 			data.pppoe.sid = act->pppoe.sid;
@@ -450,11 +453,11 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
 	if (offload_type == MTK_PPE_PKT_TYPE_BRIDGE)
 		foe.bridge.vlan = data.vlan_in;
 
-	if (data.vlan.num == 1) {
-		if (data.vlan.proto != htons(ETH_P_8021Q))
+	for (i = 0; i < data.vlan.num; i++) {
+		if (data.vlan.vlans[i].proto != htons(ETH_P_8021Q))
 			return -EOPNOTSUPP;
 
-		mtk_foe_entry_set_vlan(eth, &foe, data.vlan.id);
+		mtk_foe_entry_set_vlan(eth, &foe, data.vlan.vlans[i].id);
 	}
 	if (data.pppoe.num == 1)
 		mtk_foe_entry_set_pppoe(eth, &foe, data.pppoe.sid);
-- 
2.45.2

Re: [PATCH RFC v1 net-next] net: ethernet: mtk_ppe_offload: Allow QinQ

[Simon Horman]

On Sun, Oct 13, 2024 at 08:50:56PM +0200, Eric Woudstra wrote:
> mtk_foe_entry_set_vlan() in mtk_ppe.c already seems to support
> double vlan tagging, but mtk_flow_offload_replace() in
> mtk_ppe_offload.c only allows for 1 vlan tag, optionally in
> combination with pppoe and dsa tags.
> 
> This patch adds QinQ support to mtk_flow_offload_replace().
> 
> Only PPPoE-in-Q (as before) and Q-in-Q are allowed. A combination
> of PPPoE and Q-in-Q is not allowed.
> 
> As I do not have any documentation of the ppe hardware, I do not
> know if there is any other reason to not implement Q-in-Q in
> mtk_flow_offload_replace().
> 
> Tested on the BPI-R3(mini), on non-dsa-ports and dsa-ports.
> 
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>

Hi Eric,

I see that this patch supports up to two VLANs, both with EtherType 0x8100.
And assuming that is supported by the hardware, that seems fine to me.

But I winder if you know if this hardware supports other VLAN EtherTypes,
such as  0x88a8 which is described in 802.1ad?

...

Re: [PATCH RFC v1 net-next] net: ethernet: mtk_ppe_offload: Allow QinQ

[Eric Woudstra]

On 10/14/24 10:51 AM, Simon Horman wrote:
> On Sun, Oct 13, 2024 at 08:50:56PM +0200, Eric Woudstra wrote:
>> mtk_foe_entry_set_vlan() in mtk_ppe.c already seems to support
>> double vlan tagging, but mtk_flow_offload_replace() in
>> mtk_ppe_offload.c only allows for 1 vlan tag, optionally in
>> combination with pppoe and dsa tags.
>>
>> This patch adds QinQ support to mtk_flow_offload_replace().
>>
>> Only PPPoE-in-Q (as before) and Q-in-Q are allowed. A combination
>> of PPPoE and Q-in-Q is not allowed.
>>
>> As I do not have any documentation of the ppe hardware, I do not
>> know if there is any other reason to not implement Q-in-Q in
>> mtk_flow_offload_replace().
>>
>> Tested on the BPI-R3(mini), on non-dsa-ports and dsa-ports.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> 
> Hi Eric,
> 
> I see that this patch supports up to two VLANs, both with EtherType 0x8100.
> And assuming that is supported by the hardware, that seems fine to me.
> 
> But I winder if you know if this hardware supports other VLAN EtherTypes,
> such as  0x88a8 which is described in 802.1ad?

Hello Simon,

The issue is that I do not have the documentation. Therefore I submit
this patch as RFC and I hope someone at mediatek or so can confirm we can
implement QinQ as in this patch on all hardware that uses mtk_ppe_offload.

But it does not look like it supports 0x88a8 from 802.1ad.