| 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: <99df73ed-7988-405c-b6f9-e251ec11bb67@intel.com>
Date: Fri, 28 Feb 2025 11:15:36 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
CC: Tony Nguyen <anthony.l.nguyen@...el.com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S . Miller" <davem@...emloft.net>, "Eric
Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>, "Alexandre
Torgue" <alexandre.torgue@...s.st.com>, Simon Horman <horms@...nel.org>,
Russell King <linux@...linux.org.uk>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer
<hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, Furong Xu
<0x1207@...il.com>, Russell King <rmk+kernel@...linux.org.uk>, "Vladimir
Oltean" <vladimir.oltean@....com>, Serge Semin <fancer.lancer@...il.com>,
Xiaolei Wang <xiaolei.wang@...driver.com>, Suraj Jaiswal
<quic_jsuraj@...cinc.com>, Kory Maincent <kory.maincent@...tlin.com>, "Gal
Pressman" <gal@...dia.com>, Jesper Nilsson <jesper.nilsson@...s.com>, "Andrew
Halaney" <ahalaney@...hat.com>, Choong Yong Liang
<yong.liang.choong@...ux.intel.com>, Kunihiko Hayashi
<hayashi.kunihiko@...ionext.com>, Vinicius Costa Gomes
<vinicius.gomes@...el.com>, <intel-wired-lan@...ts.osuosl.org>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>,
<linux-arm-kernel@...ts.infradead.org>, <bpf@...r.kernel.org>
Subject: Re: [PATCH iwl-next v6 5/9] igc: Add support for frame preemption
verification
On 2/27/25 15:01, Faizal Rahim wrote:
> This patch implements the "ethtool --set-mm" callback to trigger the
> frame preemption verification handshake.
>
> Uses the MAC Merge Software Verification (mmsv) mechanism in ethtool
> to perform the verification handshake for igc.
> The structure fpe.mmsv is set by mmsv in ethtool and should remain
> read-only for the driver.
>
[..]
Thank you for the series, please find some comments inline,
sorry for them being mostly nitpicks.
> ---
> drivers/net/ethernet/intel/igc/igc.h | 12 +-
> drivers/net/ethernet/intel/igc/igc_base.h | 1 +
> drivers/net/ethernet/intel/igc/igc_defines.h | 8 +-
> drivers/net/ethernet/intel/igc/igc_ethtool.c | 21 +++
> drivers/net/ethernet/intel/igc/igc_main.c | 53 ++++++-
> drivers/net/ethernet/intel/igc/igc_tsn.c | 147 ++++++++++++++++++-
> drivers/net/ethernet/intel/igc/igc_tsn.h | 51 +++++++
> 7 files changed, 288 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 22ecdac26cf4..705bd4739e3b 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -40,6 +40,10 @@ void igc_ethtool_set_ops(struct net_device *);
>
> #define IGC_MAX_TX_TSTAMP_REGS 4
>
> +struct fpe_t {
please use "igc_" prefix
> @@ -2068,6 +2088,7 @@ static const struct ethtool_ops igc_ethtool_ops = {
> .set_rxfh = igc_ethtool_set_rxfh,
> .get_ts_info = igc_ethtool_get_ts_info,
> .get_channels = igc_ethtool_get_channels,
> + .set_mm = igc_ethtool_set_mm,
> .set_channels = igc_ethtool_set_channels,
you have picked a wierd place on this list to plug the new op
> .get_priv_flags = igc_ethtool_get_priv_flags,
> .set_priv_flags = igc_ethtool_set_priv_flags,
> +/**
> + * igc_enable_empty_addr_recv - Enable rx of packets with all-zeroes MAC address
Rx
> + * @adapter: Pointer to the igc_adapter structure.
> + *
> + * Frame preemption verification requires that packets with the all-zeroes
> + * MAC address are allowed to be received by IGC. This function adds the
s/IGC/the driver/?
> + * all-zeroes destination address to the list of acceptable addresses.
> + *
> + * Return: 0 on success, negative value otherwise.
> + */
> +int igc_enable_empty_addr_recv(struct igc_adapter *adapter)
> +{
> + u8 empty[ETH_ALEN] = { };
the style prefered is = {}
> +
> + return igc_add_mac_filter(adapter, IGC_MAC_FILTER_TYPE_DST, empty, -1);
> +}
> +
> +void igc_disable_empty_addr_recv(struct igc_adapter *adapter)
> +{
> + u8 empty[ETH_ALEN] = { };
ditto {}
> +
> + igc_del_mac_filter(adapter, IGC_MAC_FILTER_TYPE_DST, empty);
> +}
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index f0213cfce07d..acfff3919793 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -1,10 +1,137 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright (c) 2019 Intel Corporation */
>
> +#include <linux/kernel.h>
Please don't include umbrealla headers, instdead IWYU
(Include What You Use)
linux/jump_label.h etc
> #include "igc.h"
> +#include "igc_base.h"
> #include "igc_hw.h"
> #include "igc_tsn.h"
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
> index 98ec845a86bf..7ba232ef37c9 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.h
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
> @@ -4,9 +4,60 @@
> #ifndef _IGC_TSN_H_
> #define _IGC_TSN_H_
>
> +#define SMD_FRAME_SIZE 60
> +
> +enum igc_txd_popts_type {
> + SMD_V = 0x01,
> + SMD_R = 0x02
please end enums with comma (,), unless the last entry is a COUNT/MAX/
SIZE or similar
> +};
> +
> +DECLARE_STATIC_KEY_FALSE(igc_fpe_enabled);
> +
> +void igc_fpe_init(struct igc_adapter *adapter);
> +u32 igc_fpe_get_supported_frag_size(u32 user_frag_size);
> int igc_tsn_offload_apply(struct igc_adapter *adapter);
> int igc_tsn_reset(struct igc_adapter *adapter);
> void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter);
> bool igc_tsn_is_taprio_activated_by_user(struct igc_adapter *adapter);
>
> +static inline bool igc_fpe_is_pmac_enabled(struct igc_adapter *adapter)
> +{
> + return static_branch_unlikely(&igc_fpe_enabled) &&
> + adapter->fpe.mmsv.pmac_enabled;
> +}
> +
> +static inline bool igc_fpe_is_verify_or_response(union igc_adv_rx_desc *rx_desc,
> + unsigned int size)
> +{
> + u32 status_error = le32_to_cpu(rx_desc->wb.upper.status_error);
> + int smd;
> +
> + smd = FIELD_GET(IGC_RXDADV_STAT_SMD_TYPE_MASK, status_error);
> +
> + return ((smd == IGC_RXD_STAT_SMD_TYPE_V || smd == IGC_RXD_STAT_SMD_TYPE_R) &&
> + size == SMD_FRAME_SIZE);
too much braces
Powered by blists - more mailing lists