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;
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     Horatiu Vultur <horatiu.vultur@microchip.com>
Cc:     Alexander Lobakin <alexandr.lobakin@intel.com>,
        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
Date:   Wed, 16 Nov 2022 16:45:28 +0100
Message-Id: <20221116154528.3390307-1-alexandr.lobakin@intel.com>
In-Reply-To: <20221115214456.1456856-2-horatiu.vultur@microchip.com>
References: <20221115214456.1456856-1-horatiu.vultur@microchip.com> <20221115214456.1456856-2-horatiu.vultur@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 8bit
Precedence: bulk

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?
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

>  	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