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: <ZQM8wAlV50KxPjd3@lincoln> Date: Thu, 14 Sep 2023 19:02:56 +0200 From: Larysa Zaremba <larysa.zaremba@...el.com> To: Alexander Lobakin <aleksander.lobakin@...el.com> CC: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>, <andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>, <john.fastabend@...il.com>, <kpsingh@...nel.org>, <sdf@...gle.com>, <haoluo@...gle.com>, <jolsa@...nel.org>, David Ahern <dsahern@...il.com>, Jakub Kicinski <kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>, Jesper Dangaard Brouer <brouer@...hat.com>, Anatoly Burakov <anatoly.burakov@...el.com>, Alexander Lobakin <alexandr.lobakin@...el.com>, Magnus Karlsson <magnus.karlsson@...il.com>, Maryam Tahhan <mtahhan@...hat.com>, <xdp-hints@...-project.net>, <netdev@...r.kernel.org>, Willem de Bruijn <willemdebruijn.kernel@...il.com>, Alexei Starovoitov <alexei.starovoitov@...il.com>, Simon Horman <simon.horman@...igine.com>, Tariq Toukan <tariqt@...lanox.com>, Saeed Mahameed <saeedm@...lanox.com> Subject: Re: [xdp-hints] [RFC bpf-next 10/23] ice: Implement VLAN tag hint On Thu, Sep 14, 2023 at 06:38:04PM +0200, Alexander Lobakin wrote: > From: Larysa Zaremba <larysa.zaremba@...el.com> > Date: Thu, 14 Sep 2023 18:28:07 +0200 > > > On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote: > >> From: Larysa Zaremba <larysa.zaremba@...el.com> > >> Date: Thu, 24 Aug 2023 21:26:49 +0200 > > [...] > > >>> +static void > >>> +ice_set_rx_rings_vlan_proto(struct ice_vsi *vsi, __be16 vlan_ethertype) > >> > >> @vsi can be const (I hope). > > > > I will try to make it const. > > > >> Line can be broken on arguments, not type (I hope). > >> > > > > This is how we break the lines everywhere in this file though :/ > > I know and would really like us stop at least adding new such > occurrences when not needed :s I think with such minor stuff, it is more important to keep style consistent in files. > > > > >>> +{ > >>> + u16 i; > >>> + > >>> + ice_for_each_alloc_rxq(vsi, i) > >>> + vsi->rx_rings[i]->pkt_ctx.vlan_proto = vlan_ethertype; > >>> +} > >>> + > >>> /** > >>> * ice_set_vlan_offload_features - set VLAN offload features for the PF VSI > >>> * @vsi: PF's VSI > >>> @@ -6049,6 +6066,11 @@ ice_set_vlan_offload_features(struct ice_vsi *vsi, netdev_features_t features) > >>> if (strip_err || insert_err) > >>> return -EIO; > >>> > >>> + if (enable_stripping) > >>> + ice_set_rx_rings_vlan_proto(vsi, htons(vlan_ethertype)); > >>> + else > >>> + ice_set_rx_rings_vlan_proto(vsi, 0); > >> > >> Ternary? > > > > Would look ugly in this particular case, I think, too long expressions and no > > return values. > > ice_set_rx_rings_vlan_proto(vsi, strip ? htons(vlan_ethertype) : 0); > > ? > > [...] > > >>> - vlan_tag = ice_get_vlan_tag_from_rx_desc(rx_desc); > >>> + vlan_tci = ice_get_vlan_tci(rx_desc); > >> > >> Unrelated: I never was a fan of scattering rx_desc parsing across > >> several files, I remember I moved it to process_skb_fields() in both ice > >> (Hints series) and iavf (libie), maybe do that here as well? Or way too > >> out of context? > > > > A little bit too unrelated to the purpose of the series, but a thing we must do > > in the future. > > Sure, + > > > > >> > >>> > >>> /* pad the skb if needed, to make a valid ethernet frame */ > >>> if (eth_skb_pad(skb)) > >> > >> [...] > >> > >> Thanks, > >> Olek > > Thanks, > Olek
Powered by blists - more mailing lists