netdev - Re: [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities negotiation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6736625792e20_3379ce2948b@willemb.c.googlers.com.notmuch>
Date: Thu, 14 Nov 2024 15:49:27 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Milena Olech <milena.olech@...el.com>, 
 intel-wired-lan@...ts.osuosl.org
Cc: netdev@...r.kernel.org, 
 anthony.l.nguyen@...el.com, 
 przemyslaw.kitszel@...el.com, 
 Milena Olech <milena.olech@...el.com>, 
 Emil Tantilov <emil.s.tantilov@...el.com>, 
 Pavan Kumar Linga <pavan.kumar.linga@...el.com>, 
 Alexander Lobakin <aleksander.lobakin@...el.com>
Subject: Re: [PATCH iwl-net 07/10] idpf: add Tx timestamp capabilities
 negotiation

Milena Olech wrote:
> Tx timestamp capabilities are negotiated for the uplink Vport.
> Driver receives information about the number of available Tx timestamp
> latches, the size of Tx timestamp value and the set of indexes used
> for Tx timestamping.
> 
> Add function to get the Tx timestamp capabilities and parse the uplink
> vport flag.
> 
> Co-developed-by: Emil Tantilov <emil.s.tantilov@...el.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@...el.com>
> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Reviewed-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> Signed-off-by: Milena Olech <milena.olech@...el.com>

A few minor points. No big concerns from me.

>  struct idpf_vc_xn_manager;
>  
> +#define idpf_for_each_vport(adapter, iter) \
> +	for (struct idpf_vport **__##iter = &(adapter)->vports[0], \
> +	     *iter = *__##iter; \
> +	     __##iter < &(adapter)->vports[(adapter)->num_alloc_vports]; \
> +	     iter = *(++__##iter))
> +

Perhaps more readable to just use an int:

    for (int i = 0; iter = &(adapter)->vports[i], i < (adapter)->num_alloc_vports; i++)

>  /**
> @@ -517,6 +524,60 @@ static int idpf_ptp_create_clock(const struct idpf_adapter *adapter)
>  	return 0;
>  }
>  
> +/**
> + * idpf_ptp_release_vport_tstamp - Release the Tx timestamps trakcers for a

s/trakcers/trackers

> +/**
> + * struct idpf_ptp_tx_tstamp - Parametrs for Tx timestamping

s/Parametrs/Parameters

> + * @list_member: the list member strutcure

s/strutcure/Structure

Please use a spell checker, don't rely on reviewers.

Also, going forward, IMHO documentation can be limited to APIs and
non-obvious functions/structs/fields.