[PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[James]

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.

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;

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[Simon Horman]

On Wed, Aug 12, 2026 at 09:16:16AM +0000, James 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.
> 
> 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")

FWIW, I lean towards this being an enhancement for net-next without
a Fixes tag. Rather than a fix.

> 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.

Thanks for noting the testing.
I might have even included that in the patch description.

The above not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>


...

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[James]

On Fri, Aug 14, 2026 at 11:09:16AM +0100, Simon Horman <horms@kernel.org> wrote:
> FWIW, I lean towards this being an enhancement for net-next without
> a Fixes tag. Rather than a fix.

Hi Simon, thank you for the review! Apologise for the late reply as I was
unwell last week.

My rationale for targeting `net` was that the driver currently accepts bad
FCS frames by default, which I believe should be an incorrect existing behavior.
The `NETIF_F_RXALL` I added is intended to preserve access to that behavior
only when a user explicitly requests it.

With that said, I understand that exposing the `rx-all` control may make
this better suited for `net-next`. Do you prefer that I respin this as a
`net-next` patch (when it opens later this month), or do you think the
default-behavior correction justifies keeping it in `net`?

Regards,
James

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[Simon Horman]

On Wed, Aug 19, 2026 at 12:36:01PM +1000, James wrote:
> On Fri, Aug 14, 2026 at 11:09:16AM +0100, Simon Horman <horms@kernel.org> wrote:
> > FWIW, I lean towards this being an enhancement for net-next without
> > a Fixes tag. Rather than a fix.
> 
> Hi Simon, thank you for the review! Apologise for the late reply as I was
> unwell last week.
> 
> My rationale for targeting `net` was that the driver currently accepts bad
> FCS frames by default, which I believe should be an incorrect existing behavior.
> The `NETIF_F_RXALL` I added is intended to preserve access to that behavior
> only when a user explicitly requests it.
> 
> With that said, I understand that exposing the `rx-all` control may make
> this better suited for `net-next`. Do you prefer that I respin this as a
> `net-next` patch (when it opens later this month), or do you think the
> default-behavior correction justifies keeping it in `net`?

I do lean towards this being for net-next. But if you feel strongly
that it is for net, I won't push the matter any further.

BTW, net-next is currently closed. So if you do post it for net-next you
will need to wait for it to reopen around 31st August.

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "If AI is right about other error frame types it may be
better to program them all in one commit to avoid a behavior change
if someone needs the other bad frames later"
---
net: amd-xgbe: support receiving packets with bad FCS

The driver unconditionally set MAC_RCR.DCRCC when enabling RX, so frames
with a bad FCS were always accepted without any user control.  This patch
advertises NETIF_F_RXALL, leaves it off by default, and programs DCRCC
from the feature state in xgbe_set_features() and xgbe_enable_rx().

> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index 2de974213090c..cbae3d7ef9836 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -3400,7 +3400,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);

[Severity: Medium]
Can the DCRCC state programmed here disagree with the one programmed by
xgbe_set_features()?  Two different state sources are now used for the
same bit: xgbe_enable_rx() reads pdata->netdev->features, while
xgbe_set_features() uses its own features argument and records the result
in pdata->netdev_features.

The core only commits the new feature word after ndo_set_features()
returns:

net/core/dev.c:__netdev_update_features() {
	...
	if (dev->netdev_ops->ndo_set_features)
		err = dev->netdev_ops->ndo_set_features(dev, features);
	...
	dev->features = features;
	...
}

and xgbe_enable_rx()/xgbe_disable_rx() are also reached without RTNL from
the service work:

xgbe_service()
  phy_if.phy_status()
    xgbe_phy_link_status()
      xgbe_phy_start_data_path()
        hw_if->enable_rx()   /* xgbe_enable_rx() */

So if a link transition is processed while ethtool -K ethX rx-all on is
running, xgbe_enable_rx() reads the old netdev->features and writes DCRCC
back to the previous value.  Since the next request is compared against
pdata->netdev_features:

	if ((features & NETIF_F_RXALL) != rxall)

wouldn't the mismatch persist, with ethtool reporting one state and the
MAC doing the other?  Would keeping the requested DCRCC state in
pdata (updated under the same serialization as the register write) avoid
the two sources of truth?

Also, both paths perform unlocked read-modify-write cycles on MAC_RCR
through XGMAC_IOWRITE_BITS().  Can the write-back from xgbe_set_features()
clear the RE/CST/ACS bits that the service work just set in
xgbe_enable_rx(), leaving reception off until the next link event?

> @@ -3411,7 +3412,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 2d6d00e3689ba..cea49a1153ac5 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -1980,13 +1980,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;

[ ... ]

> +	rxall = pdata->netdev_features & NETIF_F_RXALL;

[ ... ]

> @@ -2017,6 +2018,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;
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> index 7d45ea22a02e2..6e30ef84239f9 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
> @@ -241,7 +241,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;

[Severity: Medium]
Does advertising NETIF_F_RXALL here promise more than the DCRCC-only
implementation delivers?

Documentation/networking/netdev-features.rst describes rx-all as:

	This requests that the NIC receive all possible frames, including
	errored frames (such as bad FCS, etc).

With only MAC_RCR.DCRCC wired up, bad-FCS frames get through, but other
errored frames still look unreachable.  MTL_Q_RQOMR.FEP/FUP are never
programmed, and xgbe_init() still carries the original note:

drivers/net/ethernet/amd/xgbe/xgbe-dev.c:xgbe_init() {
	...
	/*TODO: Error Packet and undersized good Packet forwarding enable
		(FEP and FUP)
	 */
	...
}

On top of that, xgbe_dev_read() turns any descriptor error summary into
RX_PACKET_ERRORS.FRAME, and xgbe_rx_poll() then drops the frame with no
NETIF_F_RXALL exemption:

drivers/net/ethernet/amd/xgbe/xgbe-drv.c:xgbe_rx_poll() {
	...
		if (error || packet->errors) {
			dev_kfree_skb(skb);
			goto next_packet;
		}
	...
}

Over-MTU frames are dropped unconditionally in the same function.
igb and ixgbe gate exactly these drops on

	!(netdev->features & NETIF_F_RXALL)

Would it make sense to gate the error and length drops on NETIF_F_RXALL
too, or to state the limitation explicitly?  The commit message says:

	Advertise NETIF_F_RXALL and disable it by default.

which reads as full rx-all support, while runt, giant, alignment/dribble
and receive-error frames still appear to be discarded.
-- 
pw-bot: cr

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[James]

On Mon, Aug 17, 2026 at 4:32:43PM -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "If AI is right about other error frame types it may be
> better to program them all in one commit to avoid a behavior change
> if someone needs the other bad frames later"
> ...

Hi Jakub, thanks a lot for your review!

Thank you for catching the possible race between the current feature update
path with the service-work RX re-enable path: `ndo_set_features()` programs
DCRCC before it commits `netdev_features`, while `xgbe_enable_rx()` reads
`netdev_features`. The non-atomic read-modify-write accesses to `MAC_RCR`
also make the final register value unreliable when those paths overlap.
Apologise that I've missed this.

Also taking Simon's earlier input into account, I think the best approach
for now is to set the default DCRCC value to 0, so that this network
controller can become more consistent with most other network controllers
and also with respect to user expectations. In particular, I would not
expect users to receive packets with bad FCS by default.

I will therefore drop the RXALL mechanism altogether for now (the updated,
trimmed down patch, will follow later). We can revisit this later and
implement proper RXALL support (under `net-next`, as Simon mentioned),
potentially including additional features beyond allowing packets with bad
FCS to come in.

Regards,
James

[PATCH net v2] net: amd-xgbe: support receiving packets with bad FCS

[James]

amd-xgbe driver currently sets the MAC_RCR.DCRCC bit whenever RX is
enabled. This disables hardware FCS validation, causing packets with
bad FCS to be accepted unconditionally.

This change unsets DCRCC so that packets with bad FCS will be dropped,
in-line with typical behaviours of many other network controllers.

Tests:
- Verified that packets with bad FCS are now dropped.
- Verified that receiving packets with bad FCS will increment the
  `rx_crc_errors` counter.

Signed-off-by: James Nugraha <aslan.jnn@gmail.com>
---
Changes in v2:
- Eliminated DCRCC toggling via RXALL: now it simply sets the
  correct default value on MAC Rx enable
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 2de974213..3ceb130a0 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -3400,7 +3400,7 @@ 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, 0);
 	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, CST, 1);
 	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, ACS, 1);
 	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, RE, 1);

Re: [PATCH net v2] net: amd-xgbe: support receiving packets with bad FCS

[Simon Horman]

On Wed, Aug 19, 2026 at 07:16:56PM +1000, James wrote:
> amd-xgbe driver currently sets the MAC_RCR.DCRCC bit whenever RX is
> enabled. This disables hardware FCS validation, causing packets with
> bad FCS to be accepted unconditionally.
> 
> This change unsets DCRCC so that packets with bad FCS will be dropped,
> in-line with typical behaviours of many other network controllers.
> 
> Tests:
> - Verified that packets with bad FCS are now dropped.
> - Verified that receiving packets with bad FCS will increment the
>   `rx_crc_errors` counter.
> 

Hi James,

I think this warrants a fixes tag
(no blank line between it and your signed-off-by line).

Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")

> Signed-off-by: James Nugraha <aslan.jnn@gmail.com>

net:amd-xgbe: discard rx packets with bad FCS

> ---
> Changes in v2:
> - Eliminated DCRCC toggling via RXALL: now it simply sets the
>   correct default value on MAC Rx enable
>  drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I think you may have forgotten to update the patch description
as per the suggestion made by David in his review of v1.

	net: amd-xgbe: discard rx packets with bad FCS

And, some points on process:

* Please post updated versions of patches
  as separate threads rather than as respones to earlier versions.

* Please make sure the author of the patch matches the signed off by line.
  You can do this by updating the from address in the email. Or
  if that is not possible add an extra From: line at exactly
  the first line of the body of the email.

Thanks!

-- 
pw-bot: changes-requested

Re: [PATCH net v2] net: amd-xgbe: support receiving packets with bad FCS

[David Laight]

On Wed, 19 Aug 2026 19:16:56 +1000
James <aslan.jnn@gmail.com> wrote:

> amd-xgbe driver currently sets the MAC_RCR.DCRCC bit whenever RX is
> enabled. This disables hardware FCS validation, causing packets with
> bad FCS to be accepted unconditionally.
> 
> This change unsets DCRCC so that packets with bad FCS will be dropped,
> in-line with typical behaviours of many other network controllers.
> 
> Tests:
> - Verified that packets with bad FCS are now dropped.
> - Verified that receiving packets with bad FCS will increment the
>   `rx_crc_errors` counter.
> 
> Signed-off-by: James Nugraha <aslan.jnn@gmail.com>
> ---
> Changes in v2:
> - Eliminated DCRCC toggling via RXALL: now it simply sets the
>   correct default value on MAC Rx enable
>  drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> index 2de974213..3ceb130a0 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
> @@ -3400,7 +3400,7 @@ 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, 0);
>  	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, CST, 1);
>  	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, ACS, 1);
>  	XGMAC_IOWRITE_BITS(pdata, MAC_RCR, RE, 1);
> 

You can delete the line in the disable function that turns it off.

But, really, you should be setting the required MAC_RCR bits in a single
write.
Doing RMW sequences on the hardware registers for each bit is really wrong.
You also need to start with a known value for all the bits, otherwise
'silly' things can happen is (say) some bios boot code set bits you don't
otherwise change.

	David

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[David Laight]

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;
> 

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[James]

Hi David, thank you for your feedback!

So on my test hardware (AMD Snowy Owl SoC), it is the former: FCS
validation is actually disabled, and packets with bad FCS are processed as
valid packets.

Concretely, with DCRCC=1 (the current behavior):

  - `xgbe_dev_read()` treats the packet as valid: the skb is passed up the
    stack normally (e.g. `tcpdump -Q in` will see the packets with
    bad FCS).
  - The RX CRC error counter is not incremented, so `ethtool -S` shows no
    error.
  - Since CST=1 (CRC stripping) is also set, the FCS bytes are stripped
    and are not visible in the captured frame from user-space, thus
    user space can't tell if the packet got bad FCS or not).

I verified this empirically: if the link partner sends two identical
packets, one with a valid FCS and one with a completely inverted FCS, both
arrive identically in `tcpdump -Q in` with no way to distinguish them, and
`ethtool -S` shows zero CRC errors.

With the patch applied (e.g. setting DCRCC=0), the bad-FCS packet is
dropped and RX CRC counter will get incremented.

Regards,
James

On Wed, Aug 19, 2026 at 12:43:07PM +0100, David Laight <david.laight.linux@gmail.com> wrote:
> 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.

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[David Laight]

On Thu, 20 Aug 2026 10:38:53 +1000
James <aslan.jnn@gmail.com> wrote:

> Hi David, thank you for your feedback!
> 
> So on my test hardware (AMD Snowy Owl SoC), it is the former: FCS
> validation is actually disabled, and packets with bad FCS are processed as
> valid packets.

So the subject should be:
net:amd-xgbe: discard rx packets with bad FCS

I'd also guess that you don't need to support NETIF_F_RXALL.

Parts of the driver I've just looked at are actually horrid.

All the RMW on MAC_RCR (etc) are just silly any may leave the hardware
with inconsistent settings, the code should really cache the value and
just do writes.
It might be sensible to set RE last as a separate write (done), but it
really does need to be cleared first.

Then there are all the function pointers - I failed to see anywhere that
would set alternate functions.

	David

> 
> Concretely, with DCRCC=1 (the current behavior):
> 
>   - `xgbe_dev_read()` treats the packet as valid: the skb is passed up the
>     stack normally (e.g. `tcpdump -Q in` will see the packets with
>     bad FCS).
>   - The RX CRC error counter is not incremented, so `ethtool -S` shows no
>     error.
>   - Since CST=1 (CRC stripping) is also set, the FCS bytes are stripped
>     and are not visible in the captured frame from user-space, thus
>     user space can't tell if the packet got bad FCS or not).
> 
> I verified this empirically: if the link partner sends two identical
> packets, one with a valid FCS and one with a completely inverted FCS, both
> arrive identically in `tcpdump -Q in` with no way to distinguish them, and
> `ethtool -S` shows zero CRC errors.
> 
> With the patch applied (e.g. setting DCRCC=0), the bad-FCS packet is
> dropped and RX CRC counter will get incremented.
> 
> Regards,
> James
> 
> On Wed, Aug 19, 2026 at 12:43:07PM +0100, David Laight <david.laight.linux@gmail.com> wrote:
> > 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.  
> 

Re: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS

[kernel test robot]

:::::: 
:::::: Manual check reason: "linux-review patch is more than 7 days old, verify it wasn't already superseded"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260812091616.35811-1-aslan.jnn@gmail.com>
References: <20260812091616.35811-1-aslan.jnn@gmail.com>
TO: James <aslan.jnn@gmail.com>
TO: Raju Rangoju <Raju.Rangoju@amd.com>
TO: Prashanth Kumar K R <PrashanthKumar.K.R@amd.com>
CC: netdev@vger.kernel.org
CC: Thomas.Lendacky@amd.com
CC: James Nugraha <aslan.jnn@gmail.com>

Hi James,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/James/net-amd-xgbe-support-receiving-packets-with-bad-FCS/20260812-091616
base:   net/main
patch link:    https://lore.kernel.org/r/20260812091616.35811-1-aslan.jnn%40gmail.com
patch subject: [PATCH net] net: amd-xgbe: support receiving packets with bad FCS
:::::: branch date: 16 hours ago
:::::: commit date: 16 hours ago
config: powerpc-randconfig-r123-20260821 (https://download.01.org/0day-ci/archive/20260822/202608220122.kg89130y-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 935bfc708590c60147a79c7df145bb6e68b1d388)
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260822/202608220122.kg89130y-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202608220122.kg89130y-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/amd/xgbe/xgbe-drv.c:2022:17: sparse: sparse: dubious: !x & y
   drivers/net/ethernet/amd/xgbe/xgbe-drv.c: note: in included file (through include/linux/tcp.h):
   include/linux/skbuff.h:2904:28: sparse: sparse: unsigned value that used to be signed checked against zero?
   include/linux/skbuff.h:2904:28: sparse: signed value source
--
>> drivers/net/ethernet/amd/xgbe/xgbe-dev.c:3403:9: sparse: sparse: dubious: !x & y

vim +2022 drivers/net/ethernet/amd/xgbe/xgbe-drv.c

1a510ccf5869a9 Lendacky, Thomas 2017-08-18  1977  
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1978  static int xgbe_set_features(struct net_device *netdev,
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1979  			     netdev_features_t features)
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1980  {
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1981  	struct xgbe_prv_data *pdata = netdev_priv(netdev);
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1982  	struct xgbe_hw_if *hw_if = &pdata->hw_if;
5a1edf2f430d12 James Nugraha    2026-08-12  1983  	netdev_features_t rxhash, rxcsum, rxvlan, rxvlan_filter, rxall;
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1984  	int ret = 0;
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1985  
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1986  	rxhash = pdata->netdev_features & NETIF_F_RXHASH;
801c62d945c612 Lendacky, Thomas 2014-06-24  1987  	rxcsum = pdata->netdev_features & NETIF_F_RXCSUM;
801c62d945c612 Lendacky, Thomas 2014-06-24  1988  	rxvlan = pdata->netdev_features & NETIF_F_HW_VLAN_CTAG_RX;
801c62d945c612 Lendacky, Thomas 2014-06-24  1989  	rxvlan_filter = pdata->netdev_features & NETIF_F_HW_VLAN_CTAG_FILTER;
5a1edf2f430d12 James Nugraha    2026-08-12  1990  	rxall = pdata->netdev_features & NETIF_F_RXALL;
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  1991  
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1992  	if ((features & NETIF_F_RXHASH) && !rxhash)
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1993  		ret = hw_if->enable_rss(pdata);
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1994  	else if (!(features & NETIF_F_RXHASH) && rxhash)
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1995  		ret = hw_if->disable_rss(pdata);
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1996  	if (ret)
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1997  		return ret;
5b9dfe299e5560 Lendacky, Thomas 2014-11-04  1998  
f04dd30f1bef1e Vishal Badole    2025-04-24  1999  	if ((features & NETIF_F_RXCSUM) && !rxcsum) {
f04dd30f1bef1e Vishal Badole    2025-04-24  2000  		hw_if->enable_sph(pdata);
f04dd30f1bef1e Vishal Badole    2025-04-24  2001  		hw_if->enable_vxlan(pdata);
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2002  		hw_if->enable_rx_csum(pdata);
f04dd30f1bef1e Vishal Badole    2025-04-24  2003  		schedule_work(&pdata->restart_work);
f04dd30f1bef1e Vishal Badole    2025-04-24  2004  	} else if (!(features & NETIF_F_RXCSUM) && rxcsum) {
f04dd30f1bef1e Vishal Badole    2025-04-24  2005  		hw_if->disable_sph(pdata);
f04dd30f1bef1e Vishal Badole    2025-04-24  2006  		hw_if->disable_vxlan(pdata);
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2007  		hw_if->disable_rx_csum(pdata);
f04dd30f1bef1e Vishal Badole    2025-04-24  2008  		schedule_work(&pdata->restart_work);
f04dd30f1bef1e Vishal Badole    2025-04-24  2009  	}
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2010  
801c62d945c612 Lendacky, Thomas 2014-06-24  2011  	if ((features & NETIF_F_HW_VLAN_CTAG_RX) && !rxvlan)
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2012  		hw_if->enable_rx_vlan_stripping(pdata);
801c62d945c612 Lendacky, Thomas 2014-06-24  2013  	else if (!(features & NETIF_F_HW_VLAN_CTAG_RX) && rxvlan)
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2014  		hw_if->disable_rx_vlan_stripping(pdata);
801c62d945c612 Lendacky, Thomas 2014-06-24  2015  
801c62d945c612 Lendacky, Thomas 2014-06-24  2016  	if ((features & NETIF_F_HW_VLAN_CTAG_FILTER) && !rxvlan_filter)
801c62d945c612 Lendacky, Thomas 2014-06-24  2017  		hw_if->enable_rx_vlan_filtering(pdata);
801c62d945c612 Lendacky, Thomas 2014-06-24  2018  	else if (!(features & NETIF_F_HW_VLAN_CTAG_FILTER) && rxvlan_filter)
801c62d945c612 Lendacky, Thomas 2014-06-24  2019  		hw_if->disable_rx_vlan_filtering(pdata);
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2020  
5a1edf2f430d12 James Nugraha    2026-08-12  2021  	if ((features & NETIF_F_RXALL) != rxall)
5a1edf2f430d12 James Nugraha    2026-08-12 @2022  		XGMAC_IOWRITE_BITS(pdata, MAC_RCR, DCRCC,
5a1edf2f430d12 James Nugraha    2026-08-12  2023  				   !!(features & NETIF_F_RXALL));
5a1edf2f430d12 James Nugraha    2026-08-12  2024  
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2025  	pdata->netdev_features = features;
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2026  
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2027  	DBGPR("<--xgbe_set_features\n");
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2028  
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2029  	return 0;
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2030  }
c5aa9e3b815645 Lendacky, Thomas 2014-06-05  2031  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki