[PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[Suman Ghosh]

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 v2:
- Added "fixes" tag

 .../net/ethernet/marvell/octeontx2/nic/otx2_flows.c |  7 +++++++
 .../net/ethernet/marvell/octeontx2/nic/otx2_tc.c    | 13 +++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
index 10e11262d48a..49ba27875111 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
@@ -871,6 +871,13 @@ static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
 			if (be16_to_cpu(fsp->m_ext.vlan_etype) != 0xFFFF)
 				return -EINVAL;
 
+			/* 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;
+
 			vlan_etype = be16_to_cpu(fsp->h_ext.vlan_etype);
 			/* Only ETH_P_8021Q and ETH_P_802AD types supported */
 			if (vlan_etype != ETH_P_8021Q &&
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) {
-- 
2.25.1

Re: [PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[Jakub Kicinski]

On Mon, 3 Jul 2023 22:30:54 +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>

Quoting documentation:

  Resending after review
  ~~~~~~~~~~~~~~~~~~~~~~

  Allow at least 24 hours to pass between postings...

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#resending-after-review

Re: [PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[Simon Horman]

On Mon, Jul 03, 2023 at 10:30:54PM +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>
> ---
> Changes since v2:
> - Added "fixes" tag
> 
>  .../net/ethernet/marvell/octeontx2/nic/otx2_flows.c |  7 +++++++
>  .../net/ethernet/marvell/octeontx2/nic/otx2_tc.c    | 13 +++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> index 10e11262d48a..49ba27875111 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> @@ -871,6 +871,13 @@ static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
>  			if (be16_to_cpu(fsp->m_ext.vlan_etype) != 0xFFFF)
>  				return -EINVAL;
>  
> +			/* 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;
> +

Hi Suman,

vlan_etype appears to be uninitialised here.

>  			vlan_etype = be16_to_cpu(fsp->h_ext.vlan_etype);

Perhaps the new check should go here.

>  			/* Only ETH_P_8021Q and ETH_P_802AD types supported */
>  			if (vlan_etype != ETH_P_8021Q &&

As noted by Jakub elsewhere, please allow 24h from the posting of v3
before posting v4.

-- 
pw-bot: changes-requested

Re: [PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[kernel test robot]

Hi Suman,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.4 next-20230706]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Suman-Ghosh/octeontx2-pf-Add-additional-check-for-MCAM-rules/20230704-010247
base:   linus/master
patch link:    https://lore.kernel.org/r/20230703170054.2152662-1-sumang%40marvell.com
patch subject: [PATCH V3] octeontx2-pf: Add additional check for MCAM rules
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20230706/202307062220.o7LjIj48-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230706/202307062220.o7LjIj48-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307062220.o7LjIj48-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:877:8: warning: variable 'vlan_etype' is uninitialized when used here [-Wuninitialized]
     877 |                         if (vlan_etype == ETH_P_8021Q && !fsp->m_ext.vlan_tci &&
         |                             ^~~~~~~~~~
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:867:17: note: initialize the variable 'vlan_etype' to silence this warning
     867 |                 u16 vlan_etype;
         |                               ^
         |                                = 0
   1 warning generated.


vim +/vlan_etype +877 drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c

   810	
   811	static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
   812				      struct npc_install_flow_req *req)
   813	{
   814		struct ethhdr *eth_mask = &fsp->m_u.ether_spec;
   815		struct ethhdr *eth_hdr = &fsp->h_u.ether_spec;
   816		struct flow_msg *pmask = &req->mask;
   817		struct flow_msg *pkt = &req->packet;
   818		u32 flow_type;
   819		int ret;
   820	
   821		flow_type = fsp->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS);
   822		switch (flow_type) {
   823		/* bits not set in mask are don't care */
   824		case ETHER_FLOW:
   825			if (!is_zero_ether_addr(eth_mask->h_source)) {
   826				ether_addr_copy(pkt->smac, eth_hdr->h_source);
   827				ether_addr_copy(pmask->smac, eth_mask->h_source);
   828				req->features |= BIT_ULL(NPC_SMAC);
   829			}
   830			if (!is_zero_ether_addr(eth_mask->h_dest)) {
   831				ether_addr_copy(pkt->dmac, eth_hdr->h_dest);
   832				ether_addr_copy(pmask->dmac, eth_mask->h_dest);
   833				req->features |= BIT_ULL(NPC_DMAC);
   834			}
   835			if (eth_hdr->h_proto) {
   836				memcpy(&pkt->etype, &eth_hdr->h_proto,
   837				       sizeof(pkt->etype));
   838				memcpy(&pmask->etype, &eth_mask->h_proto,
   839				       sizeof(pmask->etype));
   840				req->features |= BIT_ULL(NPC_ETYPE);
   841			}
   842			break;
   843		case IP_USER_FLOW:
   844		case TCP_V4_FLOW:
   845		case UDP_V4_FLOW:
   846		case SCTP_V4_FLOW:
   847		case AH_V4_FLOW:
   848		case ESP_V4_FLOW:
   849			ret = otx2_prepare_ipv4_flow(fsp, req, flow_type);
   850			if (ret)
   851				return ret;
   852			break;
   853		case IPV6_USER_FLOW:
   854		case TCP_V6_FLOW:
   855		case UDP_V6_FLOW:
   856		case SCTP_V6_FLOW:
   857		case AH_V6_FLOW:
   858		case ESP_V6_FLOW:
   859			ret = otx2_prepare_ipv6_flow(fsp, req, flow_type);
   860			if (ret)
   861				return ret;
   862			break;
   863		default:
   864			return -EOPNOTSUPP;
   865		}
   866		if (fsp->flow_type & FLOW_EXT) {
   867			u16 vlan_etype;
   868	
   869			if (fsp->m_ext.vlan_etype) {
   870				/* Partial masks not supported */
   871				if (be16_to_cpu(fsp->m_ext.vlan_etype) != 0xFFFF)
   872					return -EINVAL;
   873	
   874				/* Drop rule with vlan_etype == 802.1Q
   875				 * and vlan_id == 0 is not supported
   876				 */
 > 877				if (vlan_etype == ETH_P_8021Q && !fsp->m_ext.vlan_tci &&
   878				    fsp->ring_cookie == RX_CLS_FLOW_DISC)
   879					return -EINVAL;
   880	
   881				vlan_etype = be16_to_cpu(fsp->h_ext.vlan_etype);
   882				/* Only ETH_P_8021Q and ETH_P_802AD types supported */
   883				if (vlan_etype != ETH_P_8021Q &&
   884				    vlan_etype != ETH_P_8021AD)
   885					return -EINVAL;
   886	
   887				memcpy(&pkt->vlan_etype, &fsp->h_ext.vlan_etype,
   888				       sizeof(pkt->vlan_etype));
   889				memcpy(&pmask->vlan_etype, &fsp->m_ext.vlan_etype,
   890				       sizeof(pmask->vlan_etype));
   891	
   892				if (vlan_etype == ETH_P_8021Q)
   893					req->features |= BIT_ULL(NPC_VLAN_ETYPE_CTAG);
   894				else
   895					req->features |= BIT_ULL(NPC_VLAN_ETYPE_STAG);
   896			}
   897	
   898			if (fsp->m_ext.vlan_tci) {
   899				memcpy(&pkt->vlan_tci, &fsp->h_ext.vlan_tci,
   900				       sizeof(pkt->vlan_tci));
   901				memcpy(&pmask->vlan_tci, &fsp->m_ext.vlan_tci,
   902				       sizeof(pmask->vlan_tci));
   903				req->features |= BIT_ULL(NPC_OUTER_VID);
   904			}
   905	
   906			if (fsp->m_ext.data[1]) {
   907				if (flow_type == IP_USER_FLOW) {
   908					if (be32_to_cpu(fsp->h_ext.data[1]) != IPV4_FLAG_MORE)
   909						return -EINVAL;
   910	
   911					pkt->ip_flag = be32_to_cpu(fsp->h_ext.data[1]);
   912					pmask->ip_flag = be32_to_cpu(fsp->m_ext.data[1]);
   913					req->features |= BIT_ULL(NPC_IPFRAG_IPV4);
   914				} else if (fsp->h_ext.data[1] ==
   915						cpu_to_be32(OTX2_DEFAULT_ACTION)) {
   916					/* Not Drop/Direct to queue but use action
   917					 * in default entry
   918					 */
   919					req->op = NIX_RX_ACTION_DEFAULT;
   920				}
   921			}
   922		}
   923	
   924		if (fsp->flow_type & FLOW_MAC_EXT &&
   925		    !is_zero_ether_addr(fsp->m_ext.h_dest)) {
   926			ether_addr_copy(pkt->dmac, fsp->h_ext.h_dest);
   927			ether_addr_copy(pmask->dmac, fsp->m_ext.h_dest);
   928			req->features |= BIT_ULL(NPC_DMAC);
   929		}
   930	
   931		if (!req->features)
   932			return -EOPNOTSUPP;
   933	
   934		return 0;
   935	}
   936	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [EXT] Re: [PATCH V3] octeontx2-pf: Add additional check for MCAM rules

[Suman Ghosh]

>-----Original Message-----
>From: Simon Horman <simon.horman@corigine.com>
>Sent: Tuesday, July 4, 2023 7:17 PM
>To: Suman Ghosh <sumang@marvell.com>
>Cc: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
><gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
>Hariprasad Kelam <hkelam@marvell.com>; davem@davemloft.net;
>edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: [EXT] Re: [PATCH V3] octeontx2-pf: Add additional check for
>MCAM rules
>
>External Email
>
>----------------------------------------------------------------------
>On Mon, Jul 03, 2023 at 10:30:54PM +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>
>> ---
>> Changes since v2:
>> - Added "fixes" tag
>>
>>  .../net/ethernet/marvell/octeontx2/nic/otx2_flows.c |  7 +++++++
>>  .../net/ethernet/marvell/octeontx2/nic/otx2_tc.c    | 13
>+++++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
>> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
>> index 10e11262d48a..49ba27875111 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
>> @@ -871,6 +871,13 @@ static int otx2_prepare_flow_request(struct
>ethtool_rx_flow_spec *fsp,
>>  			if (be16_to_cpu(fsp->m_ext.vlan_etype) != 0xFFFF)
>>  				return -EINVAL;
>>
>> +			/* 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;
>> +
>
>Hi Suman,
>
>vlan_etype appears to be uninitialised here.
[Suman] yes. Moved the assignment of vlan_etype before the checkin v4. Hence no need for initialization now.
>
>>  			vlan_etype = be16_to_cpu(fsp->h_ext.vlan_etype);
>
>Perhaps the new check should go here.
[Suman] Ack.
>
>>  			/* Only ETH_P_8021Q and ETH_P_802AD types supported */
>>  			if (vlan_etype != ETH_P_8021Q &&
>
>As noted by Jakub elsewhere, please allow 24h from the posting of v3
>before posting v4.
>
>--
>pw-bot: changes-requested