Extract the helper fec_build_skb() from fec_enet_rx_queue(), so that the
code for building a skb is centralized in fec_build_skb(), which makes
the code of fec_enet_rx_queue() more concise and readable.
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
drivers/net/ethernet/freescale/fec_main.c | 106 ++++++++++++----------
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 68410cb3ef0a..7e8ac9d2a5ff 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1781,6 +1781,59 @@ static int fec_rx_error_check(struct net_device *ndev, u16 status)
return 0;
}
+static struct sk_buff *fec_build_skb(struct fec_enet_private *fep,
+ struct fec_enet_priv_rx_q *rxq,
+ struct bufdesc *bdp,
+ struct page *page, u32 len)
+{
+ struct net_device *ndev = fep->netdev;
+ struct bufdesc_ex *ebdp;
+ struct sk_buff *skb;
+
+ skb = build_skb(page_address(page),
+ PAGE_SIZE << fep->pagepool_order);
+ if (unlikely(!skb)) {
+ page_pool_recycle_direct(rxq->page_pool, page);
+ ndev->stats.rx_dropped++;
+ if (net_ratelimit())
+ netdev_err(ndev, "build_skb failed\n");
+
+ return NULL;
+ }
+
+ skb_reserve(skb, FEC_ENET_XDP_HEADROOM + fep->rx_shift);
+ skb_put(skb, len);
+ skb_mark_for_recycle(skb);
+
+ /* Get offloads from the enhanced buffer descriptor */
+ if (fep->bufdesc_ex) {
+ ebdp = (struct bufdesc_ex *)bdp;
+
+ /* If this is a VLAN packet remove the VLAN Tag */
+ if (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))
+ fec_enet_rx_vlan(ndev, skb);
+
+ /* Get receive timestamp from the skb */
+ if (fep->hwts_rx_en)
+ fec_enet_hwtstamp(fep, fec32_to_cpu(ebdp->ts),
+ skb_hwtstamps(skb));
+
+ if (fep->csum_flags & FLAG_RX_CSUM_ENABLED) {
+ if (!(ebdp->cbd_esc &
+ cpu_to_fec32(FLAG_RX_CSUM_ERROR)))
+ /* don't check it */
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ else
+ skb_checksum_none_assert(skb);
+ }
+ }
+
+ skb->protocol = eth_type_trans(skb, ndev);
+ skb_record_rx_queue(skb, rxq->bd.qid);
+
+ return skb;
+}
+
/* 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,
@@ -1796,7 +1849,6 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
struct sk_buff *skb;
ushort pkt_len;
int pkt_received = 0;
- struct bufdesc_ex *ebdp = NULL;
int index = 0;
bool need_swap = fep->quirks & FEC_QUIRK_SWAP_FRAME;
u32 data_start = FEC_ENET_XDP_HEADROOM + fep->rx_shift;
@@ -1866,24 +1918,6 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
goto rx_processing_done;
}
- /* The packet length includes FCS, but we don't want to
- * include that when passing upstream as it messes up
- * bridging applications.
- */
- skb = build_skb(page_address(page),
- PAGE_SIZE << fep->pagepool_order);
- if (unlikely(!skb)) {
- page_pool_recycle_direct(rxq->page_pool, page);
- ndev->stats.rx_dropped++;
-
- netdev_err_once(ndev, "build_skb failed!\n");
- goto rx_processing_done;
- }
-
- skb_reserve(skb, data_start);
- skb_put(skb, pkt_len - sub_len);
- skb_mark_for_recycle(skb);
-
if (unlikely(need_swap)) {
u8 *data;
@@ -1891,34 +1925,14 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
swap_buffer(data, pkt_len);
}
- /* Extract the enhanced buffer descriptor */
- ebdp = NULL;
- if (fep->bufdesc_ex)
- ebdp = (struct bufdesc_ex *)bdp;
-
- /* If this is a VLAN packet remove the 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);
-
- /* Get receive timestamp from the skb */
- if (fep->hwts_rx_en && fep->bufdesc_ex)
- fec_enet_hwtstamp(fep, fec32_to_cpu(ebdp->ts),
- skb_hwtstamps(skb));
-
- if (fep->bufdesc_ex &&
- (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) {
- if (!(ebdp->cbd_esc & cpu_to_fec32(FLAG_RX_CSUM_ERROR))) {
- /* don't check it */
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- } else {
- skb_checksum_none_assert(skb);
- }
- }
+ /* The packet length includes FCS, but we don't want to
+ * include that when passing upstream as it messes up
+ * bridging applications.
+ */
+ skb = fec_build_skb(fep, rxq, bdp, page, pkt_len - sub_len);
+ if (!skb)
+ goto rx_processing_done;
- skb_record_rx_queue(skb, queue_id);
napi_gro_receive(&fep->napi, skb);
rx_processing_done:
--
2.34.1On Tue, Jan 13, 2026 at 11:29:32AM +0800, Wei Fang wrote:
> Extract the helper fec_build_skb() from fec_enet_rx_queue(), so that the
> code for building a skb is centralized in fec_build_skb(), which makes
> the code of fec_enet_rx_queue() more concise and readable.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 106 ++++++++++++----------
> 1 file changed, 60 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 68410cb3ef0a..7e8ac9d2a5ff 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1781,6 +1781,59 @@ static int fec_rx_error_check(struct net_device *ndev, u16 status)
> return 0;
> }
>
> +static struct sk_buff *fec_build_skb(struct fec_enet_private *fep,
> + struct fec_enet_priv_rx_q *rxq,
> + struct bufdesc *bdp,
> + struct page *page, u32 len)
> +{
> + struct net_device *ndev = fep->netdev;
> + struct bufdesc_ex *ebdp;
> + struct sk_buff *skb;
> +
> + skb = build_skb(page_address(page),
> + PAGE_SIZE << fep->pagepool_order);
> + if (unlikely(!skb)) {
> + page_pool_recycle_direct(rxq->page_pool, page);
> + ndev->stats.rx_dropped++;
> + if (net_ratelimit())
> + netdev_err(ndev, "build_skb failed\n");
> +
> + return NULL;
> + }
> +
> + skb_reserve(skb, FEC_ENET_XDP_HEADROOM + fep->rx_shift);
> + skb_put(skb, len);
> + skb_mark_for_recycle(skb);
> +
> + /* Get offloads from the enhanced buffer descriptor */
> + if (fep->bufdesc_ex) {
> + ebdp = (struct bufdesc_ex *)bdp;
> +
> + /* If this is a VLAN packet remove the VLAN Tag */
> + if (ebdp->cbd_esc & cpu_to_fec32(BD_ENET_RX_VLAN))
> + fec_enet_rx_vlan(ndev, skb);
> +
> + /* Get receive timestamp from the skb */
> + if (fep->hwts_rx_en)
> + fec_enet_hwtstamp(fep, fec32_to_cpu(ebdp->ts),
> + skb_hwtstamps(skb));
> +
> + if (fep->csum_flags & FLAG_RX_CSUM_ENABLED) {
> + if (!(ebdp->cbd_esc &
> + cpu_to_fec32(FLAG_RX_CSUM_ERROR)))
> + /* don't check it */
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> + else
> + skb_checksum_none_assert(skb);
> + }
> + }
> +
> + skb->protocol = eth_type_trans(skb, ndev);
> + skb_record_rx_queue(skb, rxq->bd.qid);
> +
> + return skb;
> +}
> +
> /* 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,
> @@ -1796,7 +1849,6 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
> struct sk_buff *skb;
> ushort pkt_len;
> int pkt_received = 0;
> - struct bufdesc_ex *ebdp = NULL;
> int index = 0;
> bool need_swap = fep->quirks & FEC_QUIRK_SWAP_FRAME;
> u32 data_start = FEC_ENET_XDP_HEADROOM + fep->rx_shift;
> @@ -1866,24 +1918,6 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
> goto rx_processing_done;
> }
>
> - /* The packet length includes FCS, but we don't want to
> - * include that when passing upstream as it messes up
> - * bridging applications.
> - */
> - skb = build_skb(page_address(page),
> - PAGE_SIZE << fep->pagepool_order);
> - if (unlikely(!skb)) {
> - page_pool_recycle_direct(rxq->page_pool, page);
> - ndev->stats.rx_dropped++;
> -
> - netdev_err_once(ndev, "build_skb failed!\n");
> - goto rx_processing_done;
> - }
> -
> - skb_reserve(skb, data_start);
> - skb_put(skb, pkt_len - sub_len);
> - skb_mark_for_recycle(skb);
> -
> if (unlikely(need_swap)) {
> u8 *data;
>
> @@ -1891,34 +1925,14 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
> swap_buffer(data, pkt_len);
> }
>
Missed swap_buffer() in helper funciton()?
Frank
> - /* Extract the enhanced buffer descriptor */
> - ebdp = NULL;
> - if (fep->bufdesc_ex)
> - ebdp = (struct bufdesc_ex *)bdp;
> -
> - /* If this is a VLAN packet remove the 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);
> -
> - /* Get receive timestamp from the skb */
> - if (fep->hwts_rx_en && fep->bufdesc_ex)
> - fec_enet_hwtstamp(fep, fec32_to_cpu(ebdp->ts),
> - skb_hwtstamps(skb));
> -
> - if (fep->bufdesc_ex &&
> - (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) {
> - if (!(ebdp->cbd_esc & cpu_to_fec32(FLAG_RX_CSUM_ERROR))) {
> - /* don't check it */
> - skb->ip_summed = CHECKSUM_UNNECESSARY;
> - } else {
> - skb_checksum_none_assert(skb);
> - }
> - }
> + /* The packet length includes FCS, but we don't want to
> + * include that when passing upstream as it messes up
> + * bridging applications.
> + */
> + skb = fec_build_skb(fep, rxq, bdp, page, pkt_len - sub_len);
> + if (!skb)
> + goto rx_processing_done;
>
> - skb_record_rx_queue(skb, queue_id);
> napi_gro_receive(&fep->napi, skb);
>
> rx_processing_done:
> --
> 2.34.1
>
© 2016 - 2026 Red Hat, Inc.