On 4/25/23 10:43 AM, Jesper Dangaard Brouer wrote:
On 24/04/2023 21.17, John Fastabend wrote:
Just curious why not copy the logic from the other driver fms10k, ice, ect.
skb_set_hash(skb, le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
(IXGBE_RSS_L4_TYPES_MASK & (1ul << rss_type)) ?
PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);
Detail: This code mis-categorize (e.g. ARP) PKT_HASH_TYPE_L2 as
PKT_HASH_TYPE_L3, but as core reduces this further to one SKB bit, it
doesn't really matter.
avoiding the table logic. Do the driver folks care?
The define IXGBE_RSS_L4_TYPES_MASK becomes the "table" logic as a 1-bit
true/false table. It is a more compact table, let me know if this is
preferred.
Yes, it is really upto driver maintainer people to decide, what code is
preferred ?
>
Yeah doesn't matter much to me either way. I was just looking at code
compared to ice driver while reviewing.
My preference is to apply this patchset. We/I can easily followup and
change this to use the more compact approach later (if someone prefers).
Consistency might help imo and would avoid questions/confusion on /why/
doing it differently for igc vs some of the others.
I know net-next is "closed", but this patchset was posted prior to the
close. Plus, a number of companies are waiting for the XDP-hint for HW
RX timestamp. The support for driver stmmac is already in net-next
(commit e3f9c3e34840 ("net: stmmac: add Rx HWTS metadata to XDP receive
pkt")). Thus, it would be a help if both igc+stmmac changes land in same
kernel version, as both drivers are being evaluated by these companies.
Given merge window is open now and net-next closed, it's too late to land
(unless Dave/Jakub thinks otherwise given it touches also driver bits).
I've applied the series to bpf-next right now.
Thanks,
Daniel
