netdev - RE: [EXT] Re: [net-next PATCH V2] octeontx2-af: Tc flower offload support for inner VLAN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sat, 29 Jul 2023 17:42:05 +0000
From: Suman Ghosh <sumang@...vell.com>
To: Simon Horman <simon.horman@...igine.com>
CC: Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Geethasowjanya Akula
	<gakula@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linu Cherian
	<lcherian@...vell.com>,
        Jerin Jacob Kollanukkaran <jerinj@...vell.com>
Subject: RE: [EXT] Re: [net-next PATCH V2] octeontx2-af: Tc flower offload
 support for inner VLAN

>>  }
>>
>> +static int otx2_tc_process_vlan(struct otx2_nic *nic, struct flow_msg
>*flow_spec,
>> +				struct flow_msg *flow_mask, struct flow_rule *rule,
>> +				struct npc_install_flow_req *req, bool is_inner) {
>
>Hi Suman,
>
>Most of the code in this function seems to be moved from elsewhere.
>It might make it slightly easier to review if there was a patch that
>moved that code, then another patch that modified to support the inner
>VLAN feature.
[Suman] Okay Simon. I will push a patch set of 2 patches involving the suggested changes.
>
>> +	struct flow_match_vlan match;
>> +	u16 vlan_tci, vlan_tci_mask;
>> +
>> +	if (is_inner)
>> +		flow_rule_match_cvlan(rule, &match);
>> +	else
>> +		flow_rule_match_vlan(rule, &match);
>> +
>> +	if ((ntohs(match.key->vlan_tpid) != ETH_P_8021Q) &&
>> +	    (ntohs(match.key->vlan_tpid) != ETH_P_8021AD)) {
>
>I think eth_type_vlan() can be used here.
[Suman] Ack.
>
>> +		netdev_err(nic->netdev, "vlan tpid 0x%x not supported\n",
>> +			   ntohs(match.key->vlan_tpid));
>> +		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) {
>> +		vlan_tci = match.key->vlan_id |
>> +			   match.key->vlan_dei << 12 |
>> +			   match.key->vlan_priority << 13;
>> +
>> +		vlan_tci_mask = match.mask->vlan_id |
>> +				match.mask->vlan_dei << 12 |
>> +				match.mask->vlan_priority << 13;
>> +
>> +		if (is_inner) {
>> +			flow_spec->vlan_itci = htons(vlan_tci);
>> +			flow_mask->vlan_itci = htons(vlan_tci_mask);
>> +			req->features |= BIT_ULL(NPC_INNER_VID);
>> +		} else {
>> +			flow_spec->vlan_tci = htons(vlan_tci);
>> +			flow_mask->vlan_tci = htons(vlan_tci_mask);
>> +			req->features |= BIT_ULL(NPC_OUTER_VID);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int otx2_tc_prepare_flow(struct otx2_nic *nic, struct
>otx2_tc_flow *node,
>>  				struct flow_cls_offload *f,
>>  				struct npc_install_flow_req *req) @@ -458,6 +516,7
>@@ static int
>> otx2_tc_prepare_flow(struct otx2_nic *nic, struct otx2_tc_flow *node,
>>  	      BIT(FLOW_DISSECTOR_KEY_BASIC) |
>>  	      BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) |
>>  	      BIT(FLOW_DISSECTOR_KEY_VLAN) |
>> +	      BIT(FLOW_DISSECTOR_KEY_CVLAN) |
>>  	      BIT(FLOW_DISSECTOR_KEY_IPV4_ADDRS) |
>>  	      BIT(FLOW_DISSECTOR_KEY_IPV6_ADDRS) |
>>  	      BIT(FLOW_DISSECTOR_KEY_PORTS) | @@ -564,47 +623,19 @@ static
>> int otx2_tc_prepare_flow(struct otx2_nic *nic, struct otx2_tc_flow
>*node,
>>  	}
>>
>>  	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_VLAN)) {
>> -		struct flow_match_vlan match;
>> -		u16 vlan_tci, vlan_tci_mask;
>> -
>> -		flow_rule_match_vlan(rule, &match);
>> -
>> -		if (ntohs(match.key->vlan_tpid) != ETH_P_8021Q) {
>> -			netdev_err(nic->netdev, "vlan tpid 0x%x not supported\n",
>> -				   ntohs(match.key->vlan_tpid));
>> -			return -EOPNOTSUPP;
>> -		}
>> +		int ret;
>>
>> -		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) {
>> -			vlan_tci = match.key->vlan_id |
>> -				   match.key->vlan_dei << 12 |
>> -				   match.key->vlan_priority << 13;
>> +		ret = otx2_tc_process_vlan(nic, flow_spec, flow_mask, rule,
>req, false);
>> +		if (ret)
>> +			return ret;
>> +	}
>>
>> -			vlan_tci_mask = match.mask->vlan_id |
>> -					match.mask->vlan_dei << 12 |
>> -					match.mask->vlan_priority << 13;
>> +	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_CVLAN)) {
>> +		int ret;
>>
>> -			flow_spec->vlan_tci = htons(vlan_tci);
>> -			flow_mask->vlan_tci = htons(vlan_tci_mask);
>> -			req->features |= BIT_ULL(NPC_OUTER_VID);
>> -		}
>> +		ret = otx2_tc_process_vlan(nic, flow_spec, flow_mask, rule,
>req, true);
>> +		if (ret)
>> +			return ret;
>>  	}
>>
>>  	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_IPV4_ADDRS)) {
>> --
>> 2.25.1
>>
>>