| Message ID | 20230703170054.2152662-1-sumang@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [V3] octeontx2-pf: Add additional check for MCAM rules | expand |
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
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.
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, ð_hdr->h_proto, 837 sizeof(pkt->etype)); 838 memcpy(&pmask->etype, ð_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
>-----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
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) {
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(+)