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
| ||
|
Message-ID: <CANn89i+R4fdkbQr1u2L-upJobSM3aQOpGi6Kbbix_HPkkovnpA@mail.gmail.com> Date: Thu, 11 May 2023 14:33:44 +0200 From: Eric Dumazet <edumazet@...gle.com> To: Dumitru Ceara <dceara@...hat.com> Cc: Antoine Tenart <atenart@...nel.org>, davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, Ilya Maximets <i.maximets@....org> Subject: Re: [PATCH net-next 4/4] net: skbuff: fix l4_hash comment On Thu, May 11, 2023 at 2:10 PM Dumitru Ceara <dceara@...hat.com> wrote: > > Hi Antoine, > > On 5/11/23 11:34, Antoine Tenart wrote: > > Since commit 877d1f6291f8 ("net: Set sk_txhash from a random number") > > sk->sk_txhash is not a canonical 4-tuple hash. sk->sk_txhash is > > used in the TCP Tx path to populate skb->hash, with skb->l4_hash=1. > > With this, skb->l4_hash does not always indicate the hash is a > > "canonical 4-tuple hash over transport ports" but rather a hash from L4 > > layer to provide a uniform distribution over flows. Reword the comment > > accordingly, to avoid misunderstandings. > > But AFAIU the hash used to be a canonical 4-tuple hash and was used as > such by other components, e.g., OvS: > > https://elixir.bootlin.com/linux/latest/source/net/openvswitch/actions.c#L1069 > > It seems to me at least unfortunate that semantics change without > considering other users. The fact that we now fix the documentation > makes it seem like OvS was wrong to use the skb hash. However, before > 877d1f6291f8 ("net: Set sk_txhash from a random number") it was OK for > OvS to use the skb hash as a canonical 4-tuple hash. > I do not think we can undo stuff that was done back in 2015 Has anyone complained ? Note that skb->hash has never been considered as canonical, for obvious reasons. > Best regards, > Dumitru > > > > > Signed-off-by: Antoine Tenart <atenart@...nel.org> > > --- > > include/linux/skbuff.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > > index 738776ab8838..f54c84193b23 100644 > > --- a/include/linux/skbuff.h > > +++ b/include/linux/skbuff.h > > @@ -791,8 +791,8 @@ typedef unsigned char *sk_buff_data_t; > > * @active_extensions: active extensions (skb_ext_id types) > > * @ndisc_nodetype: router type (from link layer) > > * @ooo_okay: allow the mapping of a socket to a queue to be changed > > - * @l4_hash: indicate hash is a canonical 4-tuple hash over transport > > - * ports. > > + * @l4_hash: indicate hash is from layer 4 and provides a uniform > > + * distribution over flows. > > * @sw_hash: indicates hash was computed in software stack > > * @wifi_acked_valid: wifi_acked was set > > * @wifi_acked: whether frame was acked on wifi or not >
Powered by blists - more mailing lists