On Thu, 16 Jul 2020 21:12:51 +0200 Lorenzo Bianconi wrote:
> > > +static struct sk_buff *
> > > +mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> > > + 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;
> > > + skb_frag_t frags[MAX_SKB_FRAGS];
> > > + struct sk_buff *skb;
> > > +
> > > + memcpy(frags, sinfo->frags, sizeof(skb_frag_t) * num_frags);
> > > +
> > > + skb = build_skb(xdp->data_hard_start, PAGE_SIZE);
> > > + if (!skb)
> > > + return ERR_PTR(-ENOMEM);
> > > +
> > > + page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data));
> > > +
> > > + skb_reserve(skb, xdp->data - xdp->data_hard_start);
> > > + skb_put(skb, xdp->data_end - xdp->data);
> > > + mvneta_rx_csum(pp, desc_status, skb);
> > > +
> > > + for (i = 0; i < num_frags; i++) {
> > > + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > > + frags[i].bv_page, frags[i].bv_offset,
> > > + skb_frag_size(&frags[i]), PAGE_SIZE);
> > > + page_pool_release_page(rxq->page_pool, frags[i].bv_page);
> > > + }
> > > +
> > > + return skb;
> > > +}
> >
> > Here as well - is the plan to turn more of this function into common
> > code later on? Looks like most of this is not really driver specific.
>
> I agree. What about adding it when other drivers will add multi-buff support?
> (here we have even page_pool dependency)
I guess that's okay on the condition that you're going to be the one
adding the support to the next driver, or at least review it very
closely to make sure it's done.
In general vendors prove rather resistant to factoring code out,
the snowflakes they feel they are.
