| Message ID | 20241016-fec-cleanups-v1-13-de783bd15e6a@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: fec: cleanups, update quirk, update IRQ naming | expand |
On Wed, Oct 16, 2024 at 11:52:01PM +0200, Marc Kleine-Budde wrote: > In order to clean up of the VLAN handling, factor out the VLAN > handling into separate function fec_enet_rx_vlan(). > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/net/ethernet/freescale/fec_main.c | 32 ++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index d9415c7c16cea3fc3d91e198c21af9fe9e21747e..e14000ba85586b9cd73151e62924c3b4597bb580 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1672,6 +1672,22 @@ fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog, > return ret; > } > > +static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) > +{ > + struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > + > + if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) { > + /* Push and remove the vlan tag */ > + u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); > + > + memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); > + skb_pull(skb, VLAN_HLEN); > + __vlan_hwaccel_put_tag(skb, > + htons(ETH_P_8021Q), > + vlan_tag); > + } > +} > + > /* During a receive, the bd_rx.cur points to the current incoming buffer. > * When we update through the ring, if the next incoming buffer has > * not been given to the system, we just set the empty indicator, > @@ -1812,19 +1828,9 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget) > ebdp = (struct bufdesc_ex *)bdp; > > /* If this is a VLAN packet remove the VLAN Tag */ > - if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && > - fep->bufdesc_ex && > - (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) { > - /* Push and remove the vlan tag */ > - struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > - u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); > - > - memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); > - skb_pull(skb, VLAN_HLEN); > - __vlan_hwaccel_put_tag(skb, > - htons(ETH_P_8021Q), > - vlan_tag); > - } > + if (fep->bufdesc_ex && > + (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) > + fec_enet_rx_vlan(ndev, skb); > > skb->protocol = eth_type_trans(skb, ndev); > > > -- > 2.45.2 > >
> -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2024年10月17日 5:52 > To: Wei Fang <wei.fang@nxp.com>; Shenwei Wang <shenwei.wang@nxp.com>; > Clark Wang <xiaoning.wang@nxp.com>; David S. Miller > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Richard > Cochran <richardcochran@gmail.com> > Cc: imx@lists.linux.dev; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > kernel@pengutronix.de; Marc Kleine-Budde <mkl@pengutronix.de> > Subject: [PATCH net-next 13/13] net: fec: fec_enet_rx_queue(): factor out > VLAN handling into separate function fec_enet_rx_vlan() > > In order to clean up of the VLAN handling, factor out the VLAN > handling into separate function fec_enet_rx_vlan(). > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > drivers/net/ethernet/freescale/fec_main.c | 32 > ++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index > d9415c7c16cea3fc3d91e198c21af9fe9e21747e..e14000ba85586b9cd73151e > 62924c3b4597bb580 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1672,6 +1672,22 @@ fec_enet_run_xdp(struct fec_enet_private *fep, > struct bpf_prog *prog, > return ret; > } > > +static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) > +{ > + struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); Why not move vlan_header into the if statement? > + > + if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) { > + /* Push and remove the vlan tag */ > + u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); > + > + memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); > + skb_pull(skb, VLAN_HLEN); > + __vlan_hwaccel_put_tag(skb, > + htons(ETH_P_8021Q), > + vlan_tag); > + } > +} > + > /* During a receive, the bd_rx.cur points to the current incoming buffer. > * When we update through the ring, if the next incoming buffer has > * not been given to the system, we just set the empty indicator, > @@ -1812,19 +1828,9 @@ fec_enet_rx_queue(struct net_device *ndev, u16 > queue_id, int budget) > ebdp = (struct bufdesc_ex *)bdp; > > /* If this is a VLAN packet remove the VLAN Tag */ > - if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && > - fep->bufdesc_ex && > - (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) { > - /* Push and remove the vlan tag */ > - struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > - u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); > - > - memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); > - skb_pull(skb, VLAN_HLEN); > - __vlan_hwaccel_put_tag(skb, > - htons(ETH_P_8021Q), > - vlan_tag); > - } > + if (fep->bufdesc_ex && > + (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) > + fec_enet_rx_vlan(ndev, skb); > > skb->protocol = eth_type_trans(skb, ndev); > > > -- > 2.45.2 >
On 17.10.2024 03:33:09, Wei Fang wrote: > > -----Original Message----- > > From: Marc Kleine-Budde <mkl@pengutronix.de> > > Sent: 2024年10月17日 5:52 > > To: Wei Fang <wei.fang@nxp.com>; Shenwei Wang <shenwei.wang@nxp.com>; > > Clark Wang <xiaoning.wang@nxp.com>; David S. Miller > > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub > > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Richard > > Cochran <richardcochran@gmail.com> > > Cc: imx@lists.linux.dev; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > > kernel@pengutronix.de; Marc Kleine-Budde <mkl@pengutronix.de> > > Subject: [PATCH net-next 13/13] net: fec: fec_enet_rx_queue(): factor out > > VLAN handling into separate function fec_enet_rx_vlan() > > > > In order to clean up of the VLAN handling, factor out the VLAN > > handling into separate function fec_enet_rx_vlan(). > > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > --- > > drivers/net/ethernet/freescale/fec_main.c | 32 > > ++++++++++++++++++------------- > > 1 file changed, 19 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > > b/drivers/net/ethernet/freescale/fec_main.c > > index > > d9415c7c16cea3fc3d91e198c21af9fe9e21747e..e14000ba85586b9cd73151e > > 62924c3b4597bb580 100644 > > --- a/drivers/net/ethernet/freescale/fec_main.c > > +++ b/drivers/net/ethernet/freescale/fec_main.c > > @@ -1672,6 +1672,22 @@ fec_enet_run_xdp(struct fec_enet_private *fep, > > struct bpf_prog *prog, > > return ret; > > } > > > > +static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) > > +{ > > + struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > > Why not move vlan_header into the if statement? I've an upcoming patch that adds NETIF_F_HW_VLAN_STAG_RX (a.k.a. 801.2ad, S-VLAN) handling that changes this function. One hunk looks like this, it uses the vlan_header outside of the if: @@ -1675,15 +1678,19 @@ fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog, static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) { struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); + __be16 vlan_proto = vlan_header->h_vlan_proto; - if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) { + if ((vlan_proto == htons(ETH_P_8021Q) && + ndev->features & NETIF_F_HW_VLAN_CTAG_RX) || + (vlan_proto == htons(ETH_P_8021AD) && + ndev->features & NETIF_F_HW_VLAN_STAG_RX)) { /* Push and remove the vlan tag */ u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); regards, Marc
> -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2024年10月17日 15:09 > To: Wei Fang <wei.fang@nxp.com> > Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > <xiaoning.wang@nxp.com>; David S. Miller <davem@davemloft.net>; Eric > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo > Abeni <pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>; > imx@lists.linux.dev; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > kernel@pengutronix.de > Subject: Re: RE: [PATCH net-next 13/13] net: fec: fec_enet_rx_queue(): factor > out VLAN handling into separate function fec_enet_rx_vlan() > > On 17.10.2024 03:33:09, Wei Fang wrote: > > > -----Original Message----- > > > From: Marc Kleine-Budde <mkl@pengutronix.de> > > > Sent: 2024年10月17日 5:52 > > > To: Wei Fang <wei.fang@nxp.com>; Shenwei Wang > <shenwei.wang@nxp.com>; > > > Clark Wang <xiaoning.wang@nxp.com>; David S. Miller > > > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub > > > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Richard > > > Cochran <richardcochran@gmail.com> > > > Cc: imx@lists.linux.dev; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org; > > > kernel@pengutronix.de; Marc Kleine-Budde <mkl@pengutronix.de> > > > Subject: [PATCH net-next 13/13] net: fec: fec_enet_rx_queue(): factor out > > > VLAN handling into separate function fec_enet_rx_vlan() > > > > > > In order to clean up of the VLAN handling, factor out the VLAN > > > handling into separate function fec_enet_rx_vlan(). > > > > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > > --- > > > drivers/net/ethernet/freescale/fec_main.c | 32 > > > ++++++++++++++++++------------- > > > 1 file changed, 19 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > > > b/drivers/net/ethernet/freescale/fec_main.c > > > index > > > > d9415c7c16cea3fc3d91e198c21af9fe9e21747e..e14000ba85586b9cd73151e > > > 62924c3b4597bb580 100644 > > > --- a/drivers/net/ethernet/freescale/fec_main.c > > > +++ b/drivers/net/ethernet/freescale/fec_main.c > > > @@ -1672,6 +1672,22 @@ fec_enet_run_xdp(struct fec_enet_private > *fep, > > > struct bpf_prog *prog, > > > return ret; > > > } > > > > > > +static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) > > > +{ > > > + struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > > > > Why not move vlan_header into the if statement? > > I've an upcoming patch that adds NETIF_F_HW_VLAN_STAG_RX (a.k.a. > 801.2ad, S-VLAN) handling that changes this function. > > One hunk looks like this, it uses the vlan_header outside of the if: > You can move valn_header out of the 'if' when it happens. > @@ -1675,15 +1678,19 @@ fec_enet_run_xdp(struct fec_enet_private *fep, > struct bpf_prog *prog, > static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) > { > struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); > + __be16 vlan_proto = vlan_header->h_vlan_proto; > > - if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) { > + if ((vlan_proto == htons(ETH_P_8021Q) && > + ndev->features & NETIF_F_HW_VLAN_CTAG_RX) || > + (vlan_proto == htons(ETH_P_8021AD) && > + ndev->features & NETIF_F_HW_VLAN_STAG_RX)) { > /* Push and remove the vlan tag */ > u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); > > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung Nürnberg | Phone: +49-5121-206917-129 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On 17.10.2024 07:48:34, Wei Fang wrote: > > > Why not move vlan_header into the if statement? > > > > I've an upcoming patch that adds NETIF_F_HW_VLAN_STAG_RX (a.k.a. > > 801.2ad, S-VLAN) handling that changes this function. > > > > One hunk looks like this, it uses the vlan_header outside of the if: > > > > You can move valn_header out of the 'if' when it happens. Ok. regards, Marc
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index d9415c7c16cea3fc3d91e198c21af9fe9e21747e..e14000ba85586b9cd73151e62924c3b4597bb580 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1672,6 +1672,22 @@ fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog, return ret; } +static void fec_enet_rx_vlan(struct net_device *ndev, struct sk_buff *skb) +{ + struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); + + if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) { + /* Push and remove the vlan tag */ + u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); + + memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); + skb_pull(skb, VLAN_HLEN); + __vlan_hwaccel_put_tag(skb, + htons(ETH_P_8021Q), + vlan_tag); + } +} + /* During a receive, the bd_rx.cur points to the current incoming buffer. * When we update through the ring, if the next incoming buffer has * not been given to the system, we just set the empty indicator, @@ -1812,19 +1828,9 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget) ebdp = (struct bufdesc_ex *)bdp; /* If this is a VLAN packet remove the VLAN Tag */ - if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && - fep->bufdesc_ex && - (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) { - /* Push and remove the vlan tag */ - struct vlan_ethhdr *vlan_header = skb_vlan_eth_hdr(skb); - u16 vlan_tag = ntohs(vlan_header->h_vlan_TCI); - - memmove(skb->data + VLAN_HLEN, skb->data, ETH_ALEN * 2); - skb_pull(skb, VLAN_HLEN); - __vlan_hwaccel_put_tag(skb, - htons(ETH_P_8021Q), - vlan_tag); - } + if (fep->bufdesc_ex && + (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))) + fec_enet_rx_vlan(ndev, skb); skb->protocol = eth_type_trans(skb, ndev);
In order to clean up of the VLAN handling, factor out the VLAN handling into separate function fec_enet_rx_vlan(). Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- drivers/net/ethernet/freescale/fec_main.c | 32 ++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)