diff mbox series

[net,V4] octeontx2-pf: Add additional check for MCAM rules

Message ID 20230707035321.2227742-1-sumang@marvell.com
State New
Headers
Series [net,V4] octeontx2-pf: Add additional check for MCAM rules |

Commit Message

Suman Ghosh July 7, 2023, 3:53 a.m. UTC 
  Due to hardware limitation, MCAM drop rule with
ether_type == 802.1Q and vlan_id == 0 is not supported. Hence rejecting
such rules.

Fixes: dce677da57c0 ("octeontx2-pf: Add vlan-etype to ntuple filters")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
Changes since v3:
- moved assignment of vlan_etype before the if check
  
 .../net/ethernet/marvell/octeontx2/nic/otx2_flows.c |  8 ++++++++
 .../net/ethernet/marvell/octeontx2/nic/otx2_tc.c    | 13 +++++++++++++
 2 files changed, 21 insertions(+)

Comments

Michal Kubiak July 7, 2023, 7:01 p.m. UTC | #1 
On Fri, Jul 07, 2023 at 09:23:21AM +0530, Suman Ghosh wrote:
> Due to hardware limitation, MCAM drop rule with
> ether_type == 802.1Q and vlan_id == 0 is not supported. Hence rejecting
> such rules.
> 
> Fixes: dce677da57c0 ("octeontx2-pf: Add vlan-etype to ntuple filters")
> Signed-off-by: Suman Ghosh <sumang@marvell.com>

(...)

> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> @@ -604,6 +604,19 @@ static int otx2_tc_prepare_flow(struct otx2_nic *nic, struct otx2_tc_flow *node,
>  			return -EOPNOTSUPP;
>  		}
>  
> +		if (!match.mask->vlan_id) {
> +			struct flow_action_entry *act;
> +			int i;
> +
> +			flow_action_for_each(i, act, &rule->action) {
> +				if (act->id == FLOW_ACTION_DROP) {
> +					netdev_err(nic->netdev, "vlan tpid 0x%x with vlan_id %d is not supported for DROP rule.\n",
> +						   ntohs(match.key->vlan_tpid), match.key->vlan_id);
> +					return -EOPNOTSUPP;

The code got very nested here.
Please consider moving the above check to a separate helper function,
or at least reformatting the error log to fix the checkpatch warning about
80 chars, for example:

			flow_action_for_each(i, act, &rule->action) {
				if (act->id == FLOW_ACTION_DROP) {
					netdev_err(nic->netdev,
						   "vlan tpid 0x%x with vlan_id %d is not supported for DROP rule.\n",
						   ntohs(match.key->vlan_tpid),
						   match.key->vlan_id);
					return -EOPNOTSUPP;
				}

Thanks,
Michal
Suman Ghosh July 10, 2023, 10:28 a.m. UTC | #2 
>----------------------------------------------------------------------
>On Fri, Jul 07, 2023 at 09:23:21AM +0530, Suman Ghosh wrote:
>> Due to hardware limitation, MCAM drop rule with ether_type == 802.1Q
>> and vlan_id == 0 is not supported. Hence rejecting such rules.
>>
>> Fixes: dce677da57c0 ("octeontx2-pf: Add vlan-etype to ntuple filters")
>> Signed-off-by: Suman Ghosh <sumang@marvell.com>
>
>(...)
>
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
>> @@ -604,6 +604,19 @@ static int otx2_tc_prepare_flow(struct otx2_nic
>*nic, struct otx2_tc_flow *node,
>>  			return -EOPNOTSUPP;
>>  		}
>>
>> +		if (!match.mask->vlan_id) {
>> +			struct flow_action_entry *act;
>> +			int i;
>> +
>> +			flow_action_for_each(i, act, &rule->action) {
>> +				if (act->id == FLOW_ACTION_DROP) {
>> +					netdev_err(nic->netdev, "vlan tpid 0x%x with
>vlan_id %d is not supported for DROP rule.\n",
>> +						   ntohs(match.key->vlan_tpid), match.key-
>>vlan_id);
>> +					return -EOPNOTSUPP;
>
>The code got very nested here.
>Please consider moving the above check to a separate helper function, or
>at least reformatting the error log to fix the checkpatch warning about
>80 chars, for example:
[Suman] Will do the suggested reformatting in next version.
>
>			flow_action_for_each(i, act, &rule->action) {
>				if (act->id == FLOW_ACTION_DROP) {
>					netdev_err(nic->netdev,
>						   "vlan tpid 0x%x with vlan_id %d is not
>supported for DROP rule.\n",
>						   ntohs(match.key->vlan_tpid),
>						   match.key->vlan_id);
>					return -EOPNOTSUPP;
>				}
>
>Thanks,
>Michal
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
index 10e11262d48a..2d7713a1a153 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
@@ -872,6 +872,14 @@  static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
 				return -EINVAL;
 
 			vlan_etype = be16_to_cpu(fsp->h_ext.vlan_etype);
+
+			/* Drop rule with vlan_etype == 802.1Q
+			 * and vlan_id == 0 is not supported
+			 */
+			if (vlan_etype == ETH_P_8021Q && !fsp->m_ext.vlan_tci &&
+			    fsp->ring_cookie == RX_CLS_FLOW_DISC)
+				return -EINVAL;
+
 			/* Only ETH_P_8021Q and ETH_P_802AD types supported */
 			if (vlan_etype != ETH_P_8021Q &&
 			    vlan_etype != ETH_P_8021AD)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
index 044cc211424e..6c0fdc2bad73 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
@@ -604,6 +604,19 @@  static int otx2_tc_prepare_flow(struct otx2_nic *nic, struct otx2_tc_flow *node,
 			return -EOPNOTSUPP;
 		}
 
+		if (!match.mask->vlan_id) {
+			struct flow_action_entry *act;
+			int i;
+
+			flow_action_for_each(i, act, &rule->action) {
+				if (act->id == FLOW_ACTION_DROP) {
+					netdev_err(nic->netdev, "vlan tpid 0x%x with vlan_id %d is not supported for DROP rule.\n",
+						   ntohs(match.key->vlan_tpid), match.key->vlan_id);
+					return -EOPNOTSUPP;
+				}
+			}
+		}
+
 		if (match.mask->vlan_id ||
 		    match.mask->vlan_dei ||
 		    match.mask->vlan_priority) {