Re: [PATCH v8 net-next 3/4] net: enetc: add LSO support for i.MX95 ENETC PF
From: Alexander Lobakin
Date: Thu Dec 19 2024 - 10:18:02 EST
From: Wei Fang <wei.fang@xxxxxxx>
Date: Thu, 19 Dec 2024 01:32:56 +0000
>> -----Original Message-----
>> From: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
>> Sent: 2024年12月18日 22:30
>> To: Wei Fang <wei.fang@xxxxxxx>
>> Cc: Claudiu Manoil <claudiu.manoil@xxxxxxx>; Vladimir Oltean
>> <vladimir.oltean@xxxxxxx>; Clark Wang <xiaoning.wang@xxxxxxx>;
>> andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
>> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; Frank Li <frank.li@xxxxxxx>;
>> horms@xxxxxxxxxx; idosch@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx;
>> linux-kernel@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v8 net-next 3/4] net: enetc: add LSO support for i.MX95
>> ENETC PF
>>
>> From: Wei Fang <wei.fang@xxxxxxx>
>> Date: Wed, 18 Dec 2024 03:06:06 +0000
>>
>>>>> +static inline int enetc_lso_count_descs(const struct sk_buff *skb) {
>>>>> + /* 4 BDs: 1 BD for LSO header + 1 BD for extended BD + 1 BD
>>>>> + * for linear area data but not include LSO header, namely
>>>>> + * skb_headlen(skb) - lso_hdr_len. And 1 BD for gap.
>>
>> [...]
>>
>>>>> + ((first) & SILSOSFMR0_TCP_1ST_SEG))
>>>>> +
>>>>> +#define ENETC4_SILSOSFMR1 0x1304
>>>>> +#define SILSOSFMR1_TCP_LAST_SEG GENMASK(11, 0)
>>>>> +#define TCP_FLAGS_FIN BIT(0)
>>>>> +#define TCP_FLAGS_SYN BIT(1)
>>>>> +#define TCP_FLAGS_RST BIT(2)
>>>>> +#define TCP_FLAGS_PSH BIT(3)
>>>>> +#define TCP_FLAGS_ACK BIT(4)
>>>>> +#define TCP_FLAGS_URG BIT(5)
>>>>> +#define TCP_FLAGS_ECE BIT(6)
>>>>> +#define TCP_FLAGS_CWR BIT(7)
>>>>> +#define TCP_FLAGS_NS BIT(8)
>>>>
>>>> Why are you open-coding these if they're present in uapi/linux/tcp.h?
>>>
>>> Okay, I will add 'ENETC' prefix.
>>
>> You don't need to add a prefix, you need to just use the generic definitions
>> from the abovementioned file.
>
> These are definitions of register bits, they are different from the generic
> definitions. The current macros are actually different from those in tcp.h.
> The generic format is 'TCP_FLAG_XXX', while here it is 'TCP_FLAGS_XXX'.
> Anyway, I think it is better to add the 'ENETC' prefix to avoid people
> mistakenly thinking that these are generic definitions.
Oh I'm sorry, I thought those are copies of the generic defs :s
Yes, just add ENETC_ then.
Thanks,
Olek