Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> writes:
Lorenzo Bianconi <lorenzo@xxxxxxxxxx> writes:
> ...
> diff --git a/drivers/net/ethernet/marvell/mvneta.c
> b/drivers/net/ethernet/marvell/mvneta.c
> index 9d460a270601..0c7b84ca6efc 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> ...
> @@ -2320,8 +2325,12 @@ mvneta_swbm_build_skb(struct
> mvneta_port *pp,
> struct page_pool *pool,
> struct xdp_buff *xdp, u32 desc_status)
> {
> struct skb_shared_info *sinfo =
> xdp_get_shared_info_from_buff(xdp);
> - int i, num_frags = sinfo->nr_frags;
> struct sk_buff *skb;
> + u8 num_frags;
> + int i;
> +
> + if (unlikely(xdp_buff_is_mb(xdp)))
> + num_frags = sinfo->nr_frags;
Hi,
nit, it seems that the num_frags assignment can be moved after
the other
'if' condition you added (right before the 'for' for
num_frags), or even be
eliminated completely so that sinfo->nr_frags is used directly.
Either way it looks like you can remove one 'if'.
Shay
Hi Shay,
we can't move nr_frags assignement after build_skb() since this
field will be
overwritten by that call.
Regards,
Lorenzo
Sorry, silly mistake of me.
Guess this assignment can be done anyway since there doesn't seem
to be new cache misses introduced by it.
Anyway, nice catch, sorry for misleading you
> skb = build_skb(xdp->data_hard_start, PAGE_SIZE);
> if (!skb)
> @@ -2333,6 +2342,9 @@ mvneta_swbm_build_skb(struct
> mvneta_port *pp,
> struct page_pool *pool,
> skb_put(skb, xdp->data_end - xdp->data);
> skb->ip_summed = mvneta_rx_csum(pp, desc_status);
> + if (likely(!xdp_buff_is_mb(xdp)))
> + goto out;
> +
> for (i = 0; i < num_frags; i++) {
> skb_frag_t *frag = &sinfo->frags[i];
> @@ -2341,6 +2353,7 @@ mvneta_swbm_build_skb(struct
> mvneta_port *pp,
> struct page_pool *pool,
> skb_frag_size(frag), PAGE_SIZE);
> }
> +out:
> return skb;
> }
