Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 16 Nov 2022 19:50:35 +0100
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Alexander Lobakin <alexandr.lobakin@intel.com>
CC:     <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <bpf@vger.kernel.org>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <ast@kernel.org>, <daniel@iogearbox.net>, <hawk@kernel.org>,
        <john.fastabend@gmail.com>, <UNGLinuxDriver@microchip.com>
Subject: Re: [PATCH net-next v2 1/5] net: lan966x: Add XDP_PACKET_HEADROOM
Message-ID: <20221116185035.ibrby6dghiva5qdi@soft-dev3-1>
References: <20221115214456.1456856-1-horatiu.vultur@microchip.com>
 <20221115214456.1456856-2-horatiu.vultur@microchip.com>
 <20221116154528.3390307-1-alexandr.lobakin@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20221116154528.3390307-1-alexandr.lobakin@intel.com>
Precedence: bulk

The 11/16/2022 16:45, Alexander Lobakin wrote:
> [Some people who received this message don't often get email from alexandr.lobakin@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> From: Horatiu Vultur <horatiu.vultur@microchip.com>
> Date: Tue, 15 Nov 2022 22:44:52 +0100
> 
> > 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@microchip.com>
> 
> [...]
> 
> > @@ -466,6 +470,7 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
> >
> >       skb_mark_for_recycle(skb);
> >
> > +     skb_reserve(skb, XDP_PACKET_HEADROOM);
> 
> Oh, forgot to ask previously. Just curious, which platforms do
> usually have this NIC? Do those platforms have
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS set?

I am running on ARM and I can see that
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set.

> If no, then adding %NET_SKB_PAD to the headroom can significantly
> improve performance, as currently you have 28 bytes of IFH + 14
> bytes of Eth header, so IP header is not aligned to 4 bytes
> boundary. Kernel and other drivers often expect IP header to be
> aligned. Adding %NET_SKB_PAD to the headroom addresses that.
> ...but be careful, I've just realized that you have IFH in front
> of Eth header, that means that it will also become unaligned after
> that change, so make sure you don't access it with words bigger
> than 2 bytes. Just test all the variants and pick the best :D

Thanks for a detail explanation!

> 
> >       skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));
> >
> >       lan966x_ifh_get_timestamp(skb->data, &timestamp);
> > @@ -786,7 +791,8 @@ static int lan966x_fdma_get_max_frame(struct lan966x *lan966x)
> >       return lan966x_fdma_get_max_mtu(lan966x) +
> >              IFH_LEN_BYTES +
> >              SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
> > -            VLAN_HLEN * 2;
> > +            VLAN_HLEN * 2 +
> > +            XDP_PACKET_HEADROOM;
> >  }
> 
> [...]
> 
> > --
> > 2.38.0
> 
> Thanks,
> Olek

-- 
/Horatiu