The 11/14/2022 15:16, Alexander Lobakin wrote: > [Some people who received this message don't often get email from alexandr.lobakin@xxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > From: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx> > Date: Sun, 13 Nov 2022 12:15:55 +0100 Hi Olek, > > > Update the page_pool params to allocate XDP_PACKET_HEADROOM space as > > headroom for all received frames. > > This is needed for when the XDP_TX and XDP_REDIRECT are implemented. > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx> > > --- > > [...] > > > @@ -466,7 +472,8 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx, > > > > skb_mark_for_recycle(skb); > > > > - skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status)); > > + skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status) + XDP_PACKET_HEADROOM); > > + skb_pull(skb, XDP_PACKET_HEADROOM); > > These two must be: > > + skb_reserve(skb, XDP_PACKET_HEADROOM); > skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status)); > > i.e. you only need to add reserve, and that's it. It's not only > faster, but also moves ::tail, which is essential. Thanks for the suggestion. I will do that in the next version. > > > > > lan966x_ifh_get_timestamp(skb->data, ×tamp); > > > > [...] > > > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c > > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c > > @@ -44,8 +44,9 @@ int lan966x_xdp_run(struct lan966x_port *port, struct page *page, u32 data_len) > > > > xdp_init_buff(&xdp, PAGE_SIZE << lan966x->rx.page_order, > > &port->xdp_rxq); > > - xdp_prepare_buff(&xdp, page_address(page), IFH_LEN_BYTES, > > - data_len - IFH_LEN_BYTES, false); > > + xdp_prepare_buff(&xdp, page_address(page), > > + IFH_LEN_BYTES + XDP_PACKET_HEADROOM, > > + data_len - IFH_LEN_BYTES - XDP_PACKET_HEADROOM, false); > > Are you sure you need to substract %XDP_PACKET_HEADROOM from > @data_len? I think @data_len is the frame length, so headroom is not > included? Actually @data_len contains also the XDP_PACKET_HEADROOM. I am calling lan966x_xdp_run like this: --- return lan966x_xdp_run(port, page, FDMA_DCB_STATUS_BLOCKL(db->status) + XDP_PACKET_HEADROOM); --- But from what I can see, that doesn't make too much sense. Because I am adding here the XDP_PACKET_HEADROOM and then remove it in xdp_prepare_buff. So it would be better not to add it at all. > > > act = bpf_prog_run_xdp(xdp_prog, &xdp); > > switch (act) { > > case XDP_PASS: > > -- > > 2.38.0 > > Thanks, > Olek -- /Horatiu
