Re: [Intel-wired-lan] [PATCH iwl v1 0/5] igc: TX timestamping fixes

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Tony Nguyen <anthony.l.nguyen@intel.com> writes:

> On 5/4/2023 4:52 PM, Vinicius Costa Gomes wrote:
>> Hi,
>> 
>> Changes from the "for-next-queue" version:
>>   - As this is intended for the iwl/net-queue tree, removed adding
>>     support for adding the "extra" tstamp registers;
>>   - Added "Fixes:" tags to the appropriate patches (Vladimir Oltean);
>
> In most cases, net patches should have Fixes: tags to them. Patches 3 
> and 5 don't have them and it seems like it would be applicable to them.
>

Patch 3 is directly related to patch 1, but I think it deserved a
separate commit, as it has a bit of refactor. I can squash it into patch
1, if you think it's better I can do that, no worries, I was only afraid
to make the patch harder to follow.

Patch 5, as a hardware issue workaround, I didn't know if adding a
'Fixes:' tag made sense, but as a way to direct patches to the right
stable trees, that would be a good point in favor, even if it's not
fixing a bug in the code. Is this what you had in mind? If so, I can do
that.

> Patch 4 seems more like an improvement than a bug fix? If so, -next 
> would seem a better path for that patch. Based on the 'for-next-queue 
> version' link, there are still some patches remaining that will go 
> through -next? Perhaps this can go with them.
>

On a very loaded system, for example, time synchronization can fail if
something blocks the system workqueue from running, so in a sense, that
patches fixes/helps some user visible issues. But I can see it both
ways, that this is an improvement. What's your preference?

>>   - Improved the check to catch the case that the skb has the
>>     SKBTX_HW_TSTAMP flag, but TX timestamping is not enabled (Vladimir
>>     Oltean);
>>   - Ony check for timestamping timeouts if TX timestamping is enabled
>>     (Vladimir Oltean);
>> 
>> for-next-queue version link:
>> https://lore.kernel.org/intel-wired-lan/20230228054534.1093483-1-vinicius.gomes@intel.com/
>
> ...
>
>> BTW: I hope this is the correct usage of the "iwl" subject prefix.
>
> If you could also add -net|-next for the (eventual) target tree
> i.e.
>      net : iwl-net
>      net-next : iwl-next
>
> in this case 'iwl-net'

Yeah, I sent this patch a couple minutes before seeing the email about
the subject prefix conventions. Will use the correct one next time.

>
> Thanks,
> Tony

-- 
Vinicius
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan