From: David Laight <david.laight.linux@gmail.com>
To: James <aslan.jnn@gmail.com>
Cc: Raju Rangoju <Raju.Rangoju@amd.com>,
Prashanth Kumar K R <PrashanthKumar.K.R@amd.com>,
netdev@vger.kernel.org, Thomas.Lendacky@amd.com
Subject: Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS
Date: Wed, 19 Aug 2026 12:43:07 +0100 [thread overview]
Message-ID: <20260819124307.7a9ec5e6@pumpkin> (raw)
In-Reply-To: <20260812091616.35811-1-aslan.jnn@gmail.com>
On Wed, 12 Aug 2026 09:16:16 +0000
James <aslan.jnn@gmail.com> wrote:
> From: James Nugraha <aslan.jnn@gmail.com>
>
> The driver currently sets the MAC_RCR.DCRCC bit whenever RX is enabled.
> This disables hardware FCS validation, causing packets with a bad FCS to
> be accepted unconditionally. Users cannot control this behavior because
> the driver does not advertise NETIF_F_RXALL.
What actually happens to packets with bad FCS?
If FCS validation is actually disabled they'd get processed as valid packets.
That would be a serious bug.
OTOH the rx status could contain an 'fcs error' bit that causes the packet
be discarded and the software counts an error.
That would match the historic behaviour of many ethernet chips.
So neither the subject nor that text seems to be accurate.
With the bad packet (including its fcs) you can correct a single 18-bit error
burst - but I don't think anyone ever does that.
David
>
> Advertise NETIF_F_RXALL and disable it by default. Update DCRCC when the
> RXALL feature is enabled or disabled, and preserve the selected state
> across RX and link stop/start cycles.
>
> Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
> Signed-off-by: James Nugraha <aslan.jnn@gmail.com>
> ---
> Tests:
> - Verified invalid-FCS packets are dropped with RXALL disabled.
> - Verified invalid-FCS packets are received with RXALL enabled.
> - Verified invalid-FCS packets are dropped again after RXALL is disabled.
> - Verified the RXALL setting survives RX stop/start.
> - Verified the RXALL setting survives link down/up.
> - Verified RXALL is disabled by default.
>
> drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 3 +--
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 7 ++++++-
> drivers/net/ethernet/amd/xgbe/xgbe-main.c | 7 +++++--
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index 1f350d3bd..ad2030517 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -3455,7 +3455,8 @@ static void xgbe_enable_rx(struct xgbe_prv_data *pdata)
> XGMAC_IOWRITE(pdata, MAC_RQC0R, reg_val);
>
> /* Enable MAC Rx */
> - XGMAC_IOWRITE_BITS(pdata, MAC_RCR, DCRCC, 1);
> + XGMAC_IOWRITE_BITS(pdata, MAC_RCR, DCRCC,
> + !!(pdata->netdev->features & NETIF_F_RXALL));
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, CST, 1);
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, ACS, 1);
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, RE, 1);
> @@ -3466,7 +3467,6 @@ static void xgbe_disable_rx(struct xgbe_prv_data *pdata)
> unsigned int i;
>
> /* Disable MAC Rx */
> - XGMAC_IOWRITE_BITS(pdata, MAC_RCR, DCRCC, 0);
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, CST, 0);
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, ACS, 0);
> XGMAC_IOWRITE_BITS(pdata, MAC_RCR, RE, 0);
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 3a79fd054..5fb81bea1 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -2247,13 +2247,14 @@ static int xgbe_set_features(struct net_device *netdev,
> {
> struct xgbe_prv_data *pdata = netdev_priv(netdev);
> struct xgbe_hw_if *hw_if = &pdata->hw_if;
> - netdev_features_t rxhash, rxcsum, rxvlan, rxvlan_filter;
> + netdev_features_t rxhash, rxcsum, rxvlan, rxvlan_filter, rxall;
> int ret = 0;
>
> rxhash = pdata->netdev_features & NETIF_F_RXHASH;
> rxcsum = pdata->netdev_features & NETIF_F_RXCSUM;
> rxvlan = pdata->netdev_features & NETIF_F_HW_VLAN_CTAG_RX;
> rxvlan_filter = pdata->netdev_features & NETIF_F_HW_VLAN_CTAG_FILTER;
> + rxall = pdata->netdev_features & NETIF_F_RXALL;
>
> if ((features & NETIF_F_RXHASH) && !rxhash)
> ret = hw_if->enable_rss(pdata);
> @@ -2284,6 +2285,10 @@ static int xgbe_set_features(struct net_device *netdev,
> else if (!(features & NETIF_F_HW_VLAN_CTAG_FILTER) && rxvlan_filter)
> hw_if->disable_rx_vlan_filtering(pdata);
>
> + if ((features & NETIF_F_RXALL) != rxall)
> + XGMAC_IOWRITE_BITS(pdata, MAC_RCR, DCRCC,
> + !!(features & NETIF_F_RXALL));
> +
> pdata->netdev_features = features;
>
> DBGPR("<--xgbe_set_features\n");
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> index 0e8698928..898146b67 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> @@ -351,7 +351,8 @@ int xgbe_config_netdev(struct xgbe_prv_data *pdata)
> NETIF_F_GRO |
> NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_HW_VLAN_CTAG_TX |
> - NETIF_F_HW_VLAN_CTAG_FILTER;
> + NETIF_F_HW_VLAN_CTAG_FILTER |
> + NETIF_F_RXALL;
>
> if (pdata->hw_feat.rss)
> netdev->hw_features |= NETIF_F_RXHASH;
> @@ -382,2 +382,4 @@ int xgbe_config_netdev(struct xgbe_prv_data *pdata)
> netdev->features |= netdev->hw_features;
> + /* disable RXALL by default */
> + netdev->features &= ~NETIF_F_RXALL;
> pdata->netdev_features = netdev->features;
>
prev parent reply other threads:[~2026-08-19 11:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-12 9:16 [PATCH net] net: amd-xgbe: support receiving packets with bad FCS James
2026-08-14 10:09 ` Simon Horman
2026-08-19 2:36 ` James
2026-08-19 9:01 ` Simon Horman
2026-08-17 23:32 ` Jakub Kicinski
2026-08-19 6:31 ` James
2026-08-19 9:16 ` [PATCH net v2] " James
2026-08-19 11:43 ` David Laight [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260819124307.7a9ec5e6@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=PrashanthKumar.K.R@amd.com \
--cc=Raju.Rangoju@amd.com \
--cc=Thomas.Lendacky@amd.com \
--cc=aslan.jnn@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox