[PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Selvamani Rajagopal]

When MAC-PHY appends FCS to the incoming frame, FCS,
it is removed from the frame before passing it to the stack.

Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
---
 drivers/net/ethernet/oa_tc6/oa_tc6.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/oa_tc6/oa_tc6.c b/drivers/net/ethernet/oa_tc6/oa_tc6.c
index d2b05f98765b..de5f1548139f 100644
--- a/drivers/net/ethernet/oa_tc6/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6/oa_tc6.c
@@ -786,6 +786,9 @@ static void oa_tc6_submit_rx_skb(struct oa_tc6 *tc6)
 	tc6->netdev->stats.rx_packets++;
 	tc6->netdev->stats.rx_bytes += tc6->rx_skb->len;
 
+	if ((tc6->netdev->hw_features & NETIF_F_RXFCS) != 0)
+		skb_trim(tc6->rx_skb, tc6->rx_skb->len - ETH_FCS_LEN);
+
 	netif_rx(tc6->rx_skb);
 
 	tc6->rx_skb = NULL;
-- 
2.43.0

Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Andrew Lunn]

On Fri, May 29, 2026 at 06:41:00PM +0000, Selvamani Rajagopal wrote:
> When MAC-PHY appends FCS to the incoming frame, FCS,
> it is removed from the frame before passing it to the stack.

What is missing from this commit message is an answer to the question
"Why?". Does the stack do something wrong if there is a FCS at the end
of the frame?

   Andrew

RE: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Selvamani Rajagopal]

> 
> On Fri, May 29, 2026 at 06:41:00PM +0000, Selvamani Rajagopal wrote:
> > When MAC-PHY appends FCS to the incoming frame, FCS,
> > it is removed from the frame before passing it to the stack.
> 
> What is missing from this commit message is an answer to the question
> "Why?". Does the stack do something wrong if there is a FCS at the end
> of the frame?

That was a little mystery to me as well when our SQA alerted me with the issue.
When I tested, ping worked fine without any issue even with the frames with FCS appended. 
But strangely ptp4l utility failed with invalid data size. Once we removed FCS, it worked fine.

Will add this reason in the commit message.

> 
> Andrew

Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Qingfang Deng]

On Tue, 2 Jun 2026 at 02:40:13, Selvamani Rajagopal wrote:
> But strangely ptp4l utility failed with invalid data size. Once we removed FCS, it worked fine.

This is because ptp4l misinterprets the FCS as a TLV. There are already
several discussions and patches about this issue, however the maintainer
insists that the ethernet driver needs to be fixed instead:

https://sourceforge.net/p/linuxptp/mailman/message/37858618/
https://sourceforge.net/p/linuxptp/mailman/linuxptp-devel/thread/20190205170652.476c9676%40redhat.com/#msg36579297
https://sourceforge.net/p/linuxptp/mailman/linuxptp-devel/thread/CA%2BDf%2BjcRN98GpUi%3DF_4Ls7miEPbUZYb8O-gpSw0TQoqczVqMfw%40mail.gmail.com/#msg37858491
https://sourceforge.net/p/linuxptp/mailman/linuxptp-devel/thread/8ed1dc74-bf4c-2f13-4ed0-a70d0c4512f6%40cisco.com/#msg37137818

Regards,
Qingfang

Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Andrew Lunn]

On Tue, Jun 02, 2026 at 02:40:13AM +0000, Selvamani Rajagopal wrote:
> > 
> > On Fri, May 29, 2026 at 06:41:00PM +0000, Selvamani Rajagopal wrote:
> > > When MAC-PHY appends FCS to the incoming frame, FCS,
> > > it is removed from the frame before passing it to the stack.
> > 
> > What is missing from this commit message is an answer to the question
> > "Why?". Does the stack do something wrong if there is a FCS at the end
> > of the frame?
> 
> That was a little mystery to me as well when our SQA alerted me with the issue.
> When I tested, ping worked fine without any issue even with the frames with FCS appended. 
> But strangely ptp4l utility failed with invalid data size. Once we removed FCS, it worked fine.
> 
> Will add this reason in the commit message.

Please take a look at NETIF_F_RXFCS.

       Andrew

Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[David Laight]

On Sun, 31 May 2026 16:50:35 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Fri, May 29, 2026 at 06:41:00PM +0000, Selvamani Rajagopal wrote:
> > When MAC-PHY appends FCS to the incoming frame, FCS,
> > it is removed from the frame before passing it to the stack.  

If the MAC appends the FCS to an incoming frame it must be removed
before passing the frame into the stack.

> 
> What is missing from this commit message is an answer to the question
> "Why?". Does the stack do something wrong if there is a FCS at the end
> of the frame?

Unexpected padding ought to be an error.
If anything tries to parse LLC frames (with a length not an ethertype)
then padding is only expected on short frames.

Even for IP a frame with an length that is otherwise too short
might pick up the crc bytes as valid data instead of it being
discarded as invalid.

More of a question is why request the MAC append the FCS at all.
You can use it for error correction - but no one every does.
(A 2k frame has 32-14=18 crc bits for each bit offset which means
you have an 'average chance' of finding an 18bit error burst that
will fix every crc error. If you find an 8bit burst error that matches
the crc error there is a reasonable chance it is what caused the error.
Single error bursts are the most likely errors and easy to locate.)

-- David

> 
>    Andrew
> 

RE: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Selvamani Rajagopal]

> Subject: Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX
> frame
> 
> 
[..]

 
> 
> More of a question is why request the MAC append the FCS at all.
> You can use it for error correction - but no one every does.
> (A 2k frame has 32-14=18 crc bits for each bit offset which means
> you have an 'average chance' of finding an 18bit error burst that
> will fix every crc error. If you find an 8bit burst error that matches
> the crc error there is a reasonable chance it is what caused the error.
> Single error bursts are the most likely errors and easy to locate.)

My understanding is that in TX side, MAC will pad FCS automatically. RX side, MAC may or may not strip FCS
before passing to the host. I believe our MAC doesn't strip FCS in RX side.

Here is what OPEN Alliance 10BASE-T1x MAC-PHY Serial Interface specification says

7.3 Data Transaction Protocol for Ethernet Frames 
[..]
Ethernet frames are typically transferred from the SPI host to the MAC-PHY without any padding or frame 
check sequence (FCS). The MAC will automatically pad the Ethernet frame to the minimum frame size of 64 
bytes and append a computed FCS
[..]
Similarly, the MAC-PHY will typically strip the FCS from received Ethernet frames prior to transfer to the SPI 
host. However, the Ethernet specification allows the option for the Ethernet frame to be transferred to the 
MAC client with the FCS.


> 
> -- David
> 
> >
> > Andrew
> >

Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[David Laight]

On Tue, 2 Jun 2026 15:37:06 +0000
Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com> wrote:

> > Subject: Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX
> > frame
> > 
> >   
> [..]
> 
>  
> > 
> > More of a question is why request the MAC append the FCS at all.
> > You can use it for error correction - but no one every does.
> > (A 2k frame has 32-14=18 crc bits for each bit offset which means
> > you have an 'average chance' of finding an 18bit error burst that
> > will fix every crc error. If you find an 8bit burst error that matches
> > the crc error there is a reasonable chance it is what caused the error.
> > Single error bursts are the most likely errors and easy to locate.)  
> 
> My understanding is that in TX side, MAC will pad FCS automatically.

TX pad is separate from the FCS.
The frames are padded to a minimum size so that collision detect works
on maximal length 10M coax segments.
(Or rather coax+link+coax+link+coax acting as a single collision domain.)
The same minimum frame length is used on modern networks which are all
physically point-to-point and all 'hubs' are store+forward where a minimum
packet length isn't strictly needed.

VLAN headers can give unexpected amounts of padding - VLAN didn't exist
when I was writing ethernet drivers! 

> RX side, MAC may or may not strip FCS
> before passing to the host. I believe our MAC doesn't strip FCS in RX side.
> 
> Here is what OPEN Alliance 10BASE-T1x MAC-PHY Serial Interface specification says
> 
> 7.3 Data Transaction Protocol for Ethernet Frames 
> [..]
> Ethernet frames are typically transferred from the SPI host to the MAC-PHY without any padding or frame 
> check sequence (FCS). The MAC will automatically pad the Ethernet frame to the minimum frame size of 64 
> bytes and append a computed FCS
> [..]
> Similarly, the MAC-PHY will typically strip the FCS from received Ethernet frames prior to transfer to the SPI 
> host. However, the Ethernet specification allows the option for the Ethernet frame to be transferred to the 
> MAC client with the FCS.

Is this an SPI interface to the ethernet MAC?
You might want to pass the FCS over the SPI link and then verify in software
before treating the frame as valid, deleting the FCS, and passing the frame to
the protocol stack.
This would give extra protection for errors on the SPI link that might not
otherwise be detected.
(Although if you distrust the SPI link you'd what to pass the TX FCS as well.)

Unless you are doing IP checksum offload the IP checksum will pick up TCP/UDP errors.

But if you are running ISO transport (which we did 30 years ago) there is
no other checksum. So if/when you have a VMEbus card that suffers ground
bounce when, for example, passing a repeated 0xff, 0xff, 0x00 sequence you
get corrupted files from any files transfer request.

-- David

> 
> 
> > 
> > -- David
> >   
> > >
> > > Andrew
> > >  
> 

RE: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in RX frame

[Selvamani Rajagopal]

> > > Subject: Re: [PATCH net-next v3 08/14] net: ethernet: oa_tc6: Remove FCS size in
> >
> > My understanding is that in TX side, MAC will pad FCS automatically.
> 
> TX pad is separate from the FCS.
> The frames are padded to a minimum size so that collision detect works
> on maximal length 10M coax segments.
> (Or rather coax+link+coax+link+coax acting as a single collision domain.)
> The same minimum frame length is used on modern networks which are all
> physically point-to-point and all 'hubs' are store+forward where a minimum
> packet length isn't strictly needed.
> 
> VLAN headers can give unexpected amounts of padding - VLAN didn't exist
> when I was writing ethernet drivers!

Understood and agree. Mentiong "pad FCS" may be the cause for the confusion.
MAC (at least our MAC) does PAD + FCS.

> 
> 
> Is this an SPI interface to the ethernet MAC?

Yes. Correct. Twisted pair Ethernet on one end and SPI interface on the other end.

> You might want to pass the FCS over the SPI link and then verify in software
> before treating the frame as valid, deleting the FCS, and passing the frame to
> the protocol stack.
> This would give extra protection for errors on the SPI link that might not
> otherwise be detected.
> (Although if you distrust the SPI link you'd what to pass the TX FCS as well.)
> 
> Unless you are doing IP checksum offload the IP checksum will pick up TCP/UDP errors.
> 
> But if you are running ISO transport (which we did 30 years ago) there is
> no other checksum. So if/when you have a VMEbus card that suffers ground


Our MAC does give Ethernet frame with FCS to the SPI host. That's why the modification I did to oa_tc6.c,
subtracts FCS size before giving to stack. But I didn't add FCS sanity check at the host after assembling Ethernet
frame from SPI data chunks. Doing so would take good CPU time and performance may suffer. 

At least, OA TC6 framework didn't implement that (yet)


> bounce when, for example, passing a repeated 0xff, 0xff, 0x00 sequence you
> get corrupted files from any files transfer request.
> 
> -- David
> 
> >
> >
> > >
> > > -- David
> > >
> > > >
> > > > Andrew
> > > >
> >