Re: [net-next PATCH v12 11/12] octeontx2-pf: Adds TC offload support

[Kees Bakker <kees@ijzerbout.nl>]

Op 07-11-2024 om 17:08 schreef Geetha sowjanya:
> Implements tc offload support for rvu representors.
>
> Usage example:
>
>   - Add tc rule to drop packets with vlan id 3 using port
>     representor(Rpf1vf0).
>
> 	# tc filter add dev Rpf1vf0 protocol 802.1Q parent ffff: flower
> 	   vlan_id 3 vlan_ethtype ipv4 skip_sw action drop
>
> - Redirect packets with vlan id 5 and IPv4 packets to eth1,
>    after stripping vlan header.
>
> 	# tc filter add dev Rpf1vf0 ingress protocol 802.1Q flower vlan_id 5
> 	  vlan_ethtype ipv4 skip_sw action vlan pop action mirred ingress
> 	  redirect dev eth1
>
> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> ---
>   .../marvell/octeontx2/af/rvu_npc_fs.c         |  14 ++-
>   .../ethernet/marvell/octeontx2/af/rvu_rep.c   |   4 +
>   .../marvell/octeontx2/nic/otx2_common.h       |   7 ++
>   .../marvell/octeontx2/nic/otx2_flows.c        |   5 -
>   .../ethernet/marvell/octeontx2/nic/otx2_tc.c  |  25 ++--
>   .../net/ethernet/marvell/octeontx2/nic/rep.c  | 115 ++++++++++++++++++
>   .../net/ethernet/marvell/octeontx2/nic/rep.h  |   1 +
>   7 files changed, 154 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> index 150635de2bd5..9d08fd466a43 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> @@ -1416,6 +1416,7 @@ int rvu_mbox_handler_npc_install_flow(struct rvu *rvu,
>   				      struct npc_install_flow_rsp *rsp)
>   {
>   	bool from_vf = !!(req->hdr.pcifunc & RVU_PFVF_FUNC_MASK);
> +	bool from_rep_dev = !!is_rep_dev(rvu, req->hdr.pcifunc);
>   	struct rvu_switch *rswitch = &rvu->rswitch;
>   	int blkaddr, nixlf, err;
>   	struct rvu_pfvf *pfvf;
> @@ -1472,14 +1473,19 @@ int rvu_mbox_handler_npc_install_flow(struct rvu *rvu,
>   	/* AF installing for a PF/VF */
>   	if (!req->hdr.pcifunc)
(1)
>   		target = req->vf;
> +
>   	/* PF installing for its VF */
> -	else if (!from_vf && req->vf) {
> +	if (!from_vf && req->vf && !from_rep_dev) {
(2)
>   		target = (req->hdr.pcifunc & ~RVU_PFVF_FUNC_MASK) | req->vf;
>   		pf_set_vfs_mac = req->default_rule &&
>   				(req->features & BIT_ULL(NPC_DMAC));
>   	}
> -	/* msg received from PF/VF */
> +
> +	/* Representor device installing for a representee */
> +	if (from_rep_dev && req->vf)
> +		target = req->vf;
This now makes all previous assignments to `target` useless. See (1) and (2)
You created an if-else construct with an assignment to `target` in both 
paths.
Can you please check the logic again?
>   	else
> +		/* msg received from PF/VF */
>   		target = req->hdr.pcifunc;
>   
>   	/* ignore chan_mask in case pf func is not AF, revisit later */
> [...]