* [PATCH net v3 0/3] net: stmmac: Fixes for stmmac Tx VLAN insert and EST
@ 2025-10-17 6:11 Rohan G Thomas via B4 Relay
2025-10-17 6:11 ` [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload Rohan G Thomas via B4 Relay
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu,
Rohan G Thomas, Boon Khai Ng
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
Rohan G Thomas, Matthew Gerlach
This patchset includes following fixes for stmmac Tx VLAN insert and
EST implementations:
1. Disable STAG insertion offloading, as DWMAC IPs doesn't support
offload of STAG for double VLAN packets and CTAG for single VLAN
packets when using the same register configuration. The current
configuration in the driver is undocumented and is adding an
additional 802.1Q tag with VLAN ID 0 for double VLAN packets.
2. Consider Tx VLAN offload tag length for maxSDU estimation.
3. Fix GCL bounds check
Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
---
Changes in v3:
- Commit for disabling 802.1AD tag insertion offload is introduced in
to this patchset
- Add just VLAN_HLEN to sdu_len when 802.1Q tag offload is requested
- Link to v2: https://lore.kernel.org/r/20250915-qbv-fixes-v2-0-ec90673bb7d4@altera.com
Changes in v2:
- Use GENMASK instead of BIT for clarity and consistency
- Link to v1: https://lore.kernel.org/r/20250911-qbv-fixes-v1-0-e81e9597cf1f@altera.com
---
Rohan G Thomas (3):
net: stmmac: vlan: Disable 802.1AD tag insertion offload
net: stmmac: Consider Tx VLAN offload tag length for maxSDU
net: stmmac: est: Fix GCL bounds checks
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 37 ++++++++++-------------
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 4 +--
drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c | 2 +-
3 files changed, 19 insertions(+), 24 deletions(-)
---
base-commit: cb85ca4c0a349e246cd35161088aa3689ae5c580
change-id: 20250911-qbv-fixes-4ae64de86ee7
Best regards,
--
Rohan G Thomas <rohan.g.thomas@altera.com>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-17 6:11 [PATCH net v3 0/3] net: stmmac: Fixes for stmmac Tx VLAN insert and EST Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 ` Rohan G Thomas via B4 Relay 2025-10-17 12:42 ` Russell King (Oracle) 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay 2025-10-17 6:11 ` [PATCH net v3 3/3] net: stmmac: est: Fix GCL bounds checks Rohan G Thomas via B4 Relay 2 siblings, 1 reply; 19+ messages in thread From: Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 UTC (permalink / raw) To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Rohan G Thomas From: Rohan G Thomas <rohan.g.thomas@altera.com> The DWMAC IP's VLAN tag insertion offload does not support inserting STAG (802.1AD) and CTAG (802.1Q) types in bytes 13 and 14 using the same MAC_VLAN_Incl and MAC_VLAN_Inner_Incl register configurations. Currently, MAC_VLAN_Incl is configured to offload only STAG type insertion. However, the DWMAC IP inserts a CTAG type when the inner VLAN ID field of the descriptor is not configured, and a STAG type when it is configured. This behavior is not documented and leads to inconsistent double VLAN tagging. Additionally, an unexpected CTAG with VLAN ID 0 is inserted, resulting in frames like: Frame 1: 110 bytes on wire (880 bits), 110 bytes captured (880 bits) Ethernet II, Src: <src> (<src>), Dst: <dst> (<dst>) IEEE 802.1ad, ID: 100 802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 0 (unexpected) 802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 200 Internet Protocol Version 4, Src: 192.168.4.10, Dst: 192.168.4.11 Internet Control Message Protocol To avoid this undocumented and incorrect behavior, disable 802.1AD tag insertion offload. Also, don't set CSVL bit. As per the data book, when this bit is set, S-VLAN type (0x88A8) is inserted in the 13th and 14th bytes of transmitted packets and when this bit is reset, C-VLAN type (0x8100) is inserted in the 13th and 14th bytes of transmitted packets. Fixes: 30d932279dc2 ("net: stmmac: Add support for VLAN Insertion Offload") Fixes: e94e3f3b51ce ("net: stmmac: Add support for VLAN Insertion Offload in GMAC4+") Fixes: 1d2c7a5fee31 ("net: stmmac: Refactor VLAN implementation") Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com> Reviewed-by: Boon Khai Ng <boon.khai.ng@altera.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 +++++-------------- drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c | 2 +- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, struct stmmac_tx_queue *tx_q) { - u16 tag = 0x0, inner_tag = 0x0; - u32 inner_type = 0x0; + u16 tag = 0x0; struct dma_desc *p; - if (!priv->dma_cap.vlins) + if (!priv->dma_cap.vlins || !skb_vlan_tag_present(skb)) return false; - if (!skb_vlan_tag_present(skb)) - return false; - if (skb->vlan_proto == htons(ETH_P_8021AD)) { - inner_tag = skb_vlan_tag_get(skb); - inner_type = STMMAC_VLAN_INSERT; - } tag = skb_vlan_tag_get(skb); @@ -4109,7 +4102,7 @@ static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, else p = &tx_q->dma_tx[tx_q->cur_tx]; - if (stmmac_set_desc_vlan_tag(priv, p, tag, inner_tag, inner_type)) + if (stmmac_set_desc_vlan_tag(priv, p, tag, 0x0, 0x0)) return false; stmmac_set_tx_owner(priv, p); @@ -7573,11 +7566,9 @@ int stmmac_dvr_probe(struct device *device, ndev->features |= NETIF_F_HW_VLAN_CTAG_FILTER; ndev->features |= NETIF_F_HW_VLAN_STAG_FILTER; } - if (priv->dma_cap.vlins) { + if (priv->dma_cap.vlins) ndev->features |= NETIF_F_HW_VLAN_CTAG_TX; - if (priv->dma_cap.dvlan) - ndev->features |= NETIF_F_HW_VLAN_STAG_TX; - } + #endif priv->msg_enable = netif_msg_init(debug, default_msg_level); diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c index 0b6f6228ae35db3d855d8d386c3806a007a9d176..ff02a79c00d4f52a458edde1bcc08a0895b2c1c1 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c @@ -212,7 +212,7 @@ static void vlan_enable(struct mac_device_info *hw, u32 type) value = readl(ioaddr + VLAN_INCL); value |= VLAN_VLTI; - value |= VLAN_CSVL; /* Only use SVLAN */ + value &= ~VLAN_CSVL; /* Only use CVLAN */ value &= ~VLAN_VLC; value |= (type << VLAN_VLC_SHIFT) & VLAN_VLC; writel(value, ioaddr + VLAN_INCL); -- 2.43.7 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-17 6:11 ` [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload Rohan G Thomas via B4 Relay @ 2025-10-17 12:42 ` Russell King (Oracle) 2025-10-18 1:56 ` G Thomas, Rohan 0 siblings, 1 reply; 19+ messages in thread From: Russell King (Oracle) @ 2025-10-17 12:42 UTC (permalink / raw) To: rohan.g.thomas Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel On Fri, Oct 17, 2025 at 02:11:19PM +0800, Rohan G Thomas via B4 Relay wrote: > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) > static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, > struct stmmac_tx_queue *tx_q) > { > - u16 tag = 0x0, inner_tag = 0x0; > - u32 inner_type = 0x0; > + u16 tag = 0x0; > struct dma_desc *p; #include <stdnetdevcodeformat.h> - Please maintain reverse christmas- tree order. I haven't yet referred to the databook, so there may be more comments coming next week. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-17 12:42 ` Russell King (Oracle) @ 2025-10-18 1:56 ` G Thomas, Rohan 2025-10-23 3:31 ` G Thomas, Rohan 0 siblings, 1 reply; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-18 1:56 UTC (permalink / raw) To: Russell King (Oracle) Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel Hi Russell, On 10/17/2025 6:12 PM, Russell King (Oracle) wrote: > On Fri, Oct 17, 2025 at 02:11:19PM +0800, Rohan G Thomas via B4 Relay wrote: >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >> @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) >> static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, >> struct stmmac_tx_queue *tx_q) >> { >> - u16 tag = 0x0, inner_tag = 0x0; >> - u32 inner_type = 0x0; >> + u16 tag = 0x0; >> struct dma_desc *p; > > #include <stdnetdevcodeformat.h> - Please maintain reverse christmas- > tree order. Thanks for pointing this out. I'll fix the declaration order in the next revision. > > I haven't yet referred to the databook, so there may be more comments > coming next week. > Sure! Will wait for your feedback before sending the next revision. Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-18 1:56 ` G Thomas, Rohan @ 2025-10-23 3:31 ` G Thomas, Rohan 2025-10-23 10:54 ` Russell King (Oracle) 0 siblings, 1 reply; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-23 3:31 UTC (permalink / raw) To: Russell King (Oracle) Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Ng, Boon Khai, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi Russell, On 10/18/2025 7:26 AM, G Thomas, Rohan wrote: > Hi Russell, > > On 10/17/2025 6:12 PM, Russell King (Oracle) wrote: >> On Fri, Oct 17, 2025 at 02:11:19PM +0800, Rohan G Thomas via B4 Relay wrote: >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>> @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) >>> static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, >>> struct stmmac_tx_queue *tx_q) >>> { >>> - u16 tag = 0x0, inner_tag = 0x0; >>> - u32 inner_type = 0x0; >>> + u16 tag = 0x0; >>> struct dma_desc *p; >> >> #include <stdnetdevcodeformat.h> - Please maintain reverse christmas- >> tree order. > > Thanks for pointing this out. I'll fix the declaration order in the next > revision. > >> >> I haven't yet referred to the databook, so there may be more comments >> coming next week. >> > > Sure! Will wait for your feedback before sending the next revision. Just checking in — have you had a chance to review the patch further? Or would it be okay for me to go ahead and send the next revision for review? > > Best Regards, > Rohan Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-23 3:31 ` G Thomas, Rohan @ 2025-10-23 10:54 ` Russell King (Oracle) 2025-10-24 3:03 ` G Thomas, Rohan 0 siblings, 1 reply; 19+ messages in thread From: Russell King (Oracle) @ 2025-10-23 10:54 UTC (permalink / raw) To: G Thomas, Rohan Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Ng, Boon Khai, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Thu, Oct 23, 2025 at 09:01:20AM +0530, G Thomas, Rohan wrote: > Hi Russell, > > On 10/18/2025 7:26 AM, G Thomas, Rohan wrote: > > Hi Russell, > > > > On 10/17/2025 6:12 PM, Russell King (Oracle) wrote: > > > On Fri, Oct 17, 2025 at 02:11:19PM +0800, Rohan G Thomas via B4 Relay wrote: > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 > > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) > > > > static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, > > > > struct stmmac_tx_queue *tx_q) > > > > { > > > > - u16 tag = 0x0, inner_tag = 0x0; > > > > - u32 inner_type = 0x0; > > > > + u16 tag = 0x0; > > > > struct dma_desc *p; > > > > > > #include <stdnetdevcodeformat.h> - Please maintain reverse christmas- > > > tree order. > > > > Thanks for pointing this out. I'll fix the declaration order in the next > > revision. > > > > > > > > I haven't yet referred to the databook, so there may be more comments > > > coming next week. > > > > > > > Sure! Will wait for your feedback before sending the next revision. > > Just checking in — have you had a chance to review the patch further? Or > would it be okay for me to go ahead and send the next revision for > review? I've checked my version of the databook, and the core version that has VLINS/DVLAN and my databook doesn't cover this. So I'm afraid I can't review further. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload 2025-10-23 10:54 ` Russell King (Oracle) @ 2025-10-24 3:03 ` G Thomas, Rohan 0 siblings, 0 replies; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-24 3:03 UTC (permalink / raw) To: Russell King (Oracle) Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Ng, Boon Khai, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi Russell, On 10/23/2025 4:24 PM, Russell King (Oracle) wrote: > On Thu, Oct 23, 2025 at 09:01:20AM +0530, G Thomas, Rohan wrote: >> Hi Russell, >> >> On 10/18/2025 7:26 AM, G Thomas, Rohan wrote: >>> Hi Russell, >>> >>> On 10/17/2025 6:12 PM, Russell King (Oracle) wrote: >>>> On Fri, Oct 17, 2025 at 02:11:19PM +0800, Rohan G Thomas via B4 Relay wrote: >>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>>> index 650d75b73e0b0ecd02d35dd5d6a8742d45188c47..dedaaef3208bfadc105961029f79d0d26c3289d8 100644 >>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c >>>>> @@ -4089,18 +4089,11 @@ static int stmmac_release(struct net_device *dev) >>>>> static bool stmmac_vlan_insert(struct stmmac_priv *priv, struct sk_buff *skb, >>>>> struct stmmac_tx_queue *tx_q) >>>>> { >>>>> - u16 tag = 0x0, inner_tag = 0x0; >>>>> - u32 inner_type = 0x0; >>>>> + u16 tag = 0x0; >>>>> struct dma_desc *p; >>>> >>>> #include <stdnetdevcodeformat.h> - Please maintain reverse christmas- >>>> tree order. >>> >>> Thanks for pointing this out. I'll fix the declaration order in the next >>> revision. >>> >>>> >>>> I haven't yet referred to the databook, so there may be more comments >>>> coming next week. >>>> >>> >>> Sure! Will wait for your feedback before sending the next revision. >> >> Just checking in — have you had a chance to review the patch further? Or >> would it be okay for me to go ahead and send the next revision for >> review? > > I've checked my version of the databook, and the core version that has > VLINS/DVLAN and my databook doesn't cover this. So I'm afraid I can't > review further. > Thanks for checking. Understood. Following public document appears to include the DWMAC QoS IP databook. Section 44 of the following user manual might be a useful reference. https://www.infineon.com/row/public/documents/10/44/infineon-aurix-tc3xx-part2-usermanual-en.pdf Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 6:11 [PATCH net v3 0/3] net: stmmac: Fixes for stmmac Tx VLAN insert and EST Rohan G Thomas via B4 Relay 2025-10-17 6:11 ` [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 ` Rohan G Thomas via B4 Relay 2025-10-17 7:36 ` G Thomas, Rohan ` (3 more replies) 2025-10-17 6:11 ` [PATCH net v3 3/3] net: stmmac: est: Fix GCL bounds checks Rohan G Thomas via B4 Relay 2 siblings, 4 replies; 19+ messages in thread From: Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 UTC (permalink / raw) To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Rohan G Thomas, Matthew Gerlach From: Rohan G Thomas <rohan.g.thomas@altera.com> On hardware with Tx VLAN offload enabled, add the VLAN tag length to the skb length before checking the Qbv maxSDU if Tx VLAN offload is requested for the packet. Add 4 bytes for 802.1Q tag. Fixes: c5c3e1bfc9e0 ("net: stmmac: Offload queueMaxSDU from tc-taprio") Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index dedaaef3208bfadc105961029f79d0d26c3289d8..23bf4a3d324b7f8e8c3067ed4d47b436a89c97d3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4500,6 +4500,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) bool has_vlan, set_ic; int entry, first_tx; dma_addr_t des; + u32 sdu_len; tx_q = &priv->dma_conf.tx_queue[queue]; txq_stats = &priv->xstats.txq_stats[queue]; @@ -4516,13 +4517,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) return stmmac_tso_xmit(skb, dev); } - if (priv->est && priv->est->enable && - priv->est->max_sdu[queue] && - skb->len > priv->est->max_sdu[queue]){ - priv->xstats.max_sdu_txq_drop[queue]++; - goto max_sdu_err; - } - if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) { if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) { netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, @@ -4535,8 +4529,18 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_BUSY; } + sdu_len = skb->len; /* Check if VLAN can be inserted by HW */ has_vlan = stmmac_vlan_insert(priv, skb, tx_q); + if (has_vlan) + sdu_len += VLAN_HLEN; + + if (priv->est && priv->est->enable && + priv->est->max_sdu[queue] && + skb->len > priv->est->max_sdu[queue]){ + priv->xstats.max_sdu_txq_drop[queue]++; + goto max_sdu_err; + } entry = tx_q->cur_tx; first_entry = entry; -- 2.43.7 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay @ 2025-10-17 7:36 ` G Thomas, Rohan 2025-10-17 12:21 ` Vadim Fedorenko 2025-10-17 12:44 ` Russell King (Oracle) ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-17 7:36 UTC (permalink / raw) To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi All, On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: > > + sdu_len = skb->len; > /* Check if VLAN can be inserted by HW */ > has_vlan = stmmac_vlan_insert(priv, skb, tx_q); > + if (has_vlan) > + sdu_len += VLAN_HLEN; > + > + if (priv->est && priv->est->enable && > + priv->est->max_sdu[queue] && > + skb->len > priv->est->max_sdu[queue]){ I just noticed an issue with the reworked fix after sending the patch. The condition should be: sdu_len > priv->est->max_sdu[queue] I’ll send a corrected version, and will wait for any additional comments in the meantime. > + priv->xstats.max_sdu_txq_drop[queue]++; > + goto max_sdu_err; > + } > > entry = tx_q->cur_tx; > first_entry = entry; > Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 7:36 ` G Thomas, Rohan @ 2025-10-17 12:21 ` Vadim Fedorenko 2025-10-18 1:50 ` G Thomas, Rohan 0 siblings, 1 reply; 19+ messages in thread From: Vadim Fedorenko @ 2025-10-17 12:21 UTC (permalink / raw) To: G Thomas, Rohan, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach On 17/10/2025 08:36, G Thomas, Rohan wrote: > Hi All, > > On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: >> + sdu_len = skb->len; >> /* Check if VLAN can be inserted by HW */ >> has_vlan = stmmac_vlan_insert(priv, skb, tx_q); >> + if (has_vlan) >> + sdu_len += VLAN_HLEN; >> + >> + if (priv->est && priv->est->enable && >> + priv->est->max_sdu[queue] && >> + skb->len > priv->est->max_sdu[queue]){ > > I just noticed an issue with the reworked fix after sending the patch. > The condition should be: sdu_len > priv->est->max_sdu[queue] > > I’ll send a corrected version, and will wait for any additional comments > in the meantime. Well, even though it's a copy of original code, it would be good to improve some formatting and add a space at the end of if statement to make it look like 'if () {'> >> + priv->xstats.max_sdu_txq_drop[queue]++; >> + goto max_sdu_err; >> + } >> entry = tx_q->cur_tx; >> first_entry = entry; >> > > Best Regards, > Rohan > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 12:21 ` Vadim Fedorenko @ 2025-10-18 1:50 ` G Thomas, Rohan 2025-10-23 10:59 ` Russell King (Oracle) 0 siblings, 1 reply; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-18 1:50 UTC (permalink / raw) To: Vadim Fedorenko, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi Vadim, On 10/17/2025 5:51 PM, Vadim Fedorenko wrote: > On 17/10/2025 08:36, G Thomas, Rohan wrote: >> Hi All, >> >> On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: >>> + sdu_len = skb->len; >>> /* Check if VLAN can be inserted by HW */ >>> has_vlan = stmmac_vlan_insert(priv, skb, tx_q); >>> + if (has_vlan) >>> + sdu_len += VLAN_HLEN; >>> + >>> + if (priv->est && priv->est->enable && >>> + priv->est->max_sdu[queue] && >>> + skb->len > priv->est->max_sdu[queue]){ >> >> I just noticed an issue with the reworked fix after sending the patch. >> The condition should be: sdu_len > priv->est->max_sdu[queue] >> >> I’ll send a corrected version, and will wait for any additional comments >> in the meantime. > > Well, even though it's a copy of original code, it would be good to > improve some formatting and add a space at the end of if statement to > make it look like 'if () {'> Thanks for pointing this out. I'll fix the formatting in the next version. >>> + priv->xstats.max_sdu_txq_drop[queue]++; >>> + goto max_sdu_err; >>> + } >>> entry = tx_q->cur_tx; >>> first_entry = entry; >>> >> >> Best Regards, >> Rohan >> > Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-18 1:50 ` G Thomas, Rohan @ 2025-10-23 10:59 ` Russell King (Oracle) 2025-10-23 16:03 ` G Thomas, Rohan 0 siblings, 1 reply; 19+ messages in thread From: Russell King (Oracle) @ 2025-10-23 10:59 UTC (permalink / raw) To: G Thomas, Rohan Cc: Vadim Fedorenko, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach On Sat, Oct 18, 2025 at 07:20:03AM +0530, G Thomas, Rohan wrote: > Hi Vadim, > > On 10/17/2025 5:51 PM, Vadim Fedorenko wrote: > > On 17/10/2025 08:36, G Thomas, Rohan wrote: > > > Hi All, > > > > > > On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: > > > > + sdu_len = skb->len; > > > > /* Check if VLAN can be inserted by HW */ > > > > has_vlan = stmmac_vlan_insert(priv, skb, tx_q); > > > > + if (has_vlan) > > > > + sdu_len += VLAN_HLEN; > > > > + > > > > + if (priv->est && priv->est->enable && > > > > + priv->est->max_sdu[queue] && > > > > + skb->len > priv->est->max_sdu[queue]){ > > > > > > I just noticed an issue with the reworked fix after sending the patch. > > > The condition should be: sdu_len > priv->est->max_sdu[queue] > > > > > > I’ll send a corrected version, and will wait for any additional comments > > > in the meantime. > > > > Well, even though it's a copy of original code, it would be good to > > improve some formatting and add a space at the end of if statement to > > make it look like 'if () {'> > > Thanks for pointing this out. I'll fix the formatting in the next version. I suggest: First patch - fix formatting. Second patch - move the code. We have a general rule that when code is moved, it should be moved with no changes - otherwise it makes review much harder. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-23 10:59 ` Russell King (Oracle) @ 2025-10-23 16:03 ` G Thomas, Rohan 0 siblings, 0 replies; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-23 16:03 UTC (permalink / raw) To: Russell King (Oracle) Cc: Vadim Fedorenko, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi Russell, On 10/23/2025 4:29 PM, Russell King (Oracle) wrote: > On Sat, Oct 18, 2025 at 07:20:03AM +0530, G Thomas, Rohan wrote: >> Hi Vadim, >> >> On 10/17/2025 5:51 PM, Vadim Fedorenko wrote: >>> On 17/10/2025 08:36, G Thomas, Rohan wrote: >>>> Hi All, >>>> >>>> On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: >>>>> + sdu_len = skb->len; >>>>> /* Check if VLAN can be inserted by HW */ >>>>> has_vlan = stmmac_vlan_insert(priv, skb, tx_q); >>>>> + if (has_vlan) >>>>> + sdu_len += VLAN_HLEN; >>>>> + >>>>> + if (priv->est && priv->est->enable && >>>>> + priv->est->max_sdu[queue] && >>>>> + skb->len > priv->est->max_sdu[queue]){ >>>> >>>> I just noticed an issue with the reworked fix after sending the patch. >>>> The condition should be: sdu_len > priv->est->max_sdu[queue] >>>> >>>> I’ll send a corrected version, and will wait for any additional comments >>>> in the meantime. >>> >>> Well, even though it's a copy of original code, it would be good to >>> improve some formatting and add a space at the end of if statement to >>> make it look like 'if () {'> >> >> Thanks for pointing this out. I'll fix the formatting in the next version. > > I suggest: > > First patch - fix formatting. > Second patch - move the code. > > We have a general rule that when code is moved, it should be moved with > no changes - otherwise it makes review much harder. > Thanks for the suggestion. Will do it in separate patches. > Thanks. > Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay 2025-10-17 7:36 ` G Thomas, Rohan @ 2025-10-17 12:44 ` Russell King (Oracle) 2025-10-18 2:06 ` G Thomas, Rohan 2025-10-17 17:16 ` kernel test robot 2025-10-27 9:03 ` G Thomas, Rohan 3 siblings, 1 reply; 19+ messages in thread From: Russell King (Oracle) @ 2025-10-17 12:44 UTC (permalink / raw) To: rohan.g.thomas Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach On Fri, Oct 17, 2025 at 02:11:20PM +0800, Rohan G Thomas via B4 Relay wrote: > From: Rohan G Thomas <rohan.g.thomas@altera.com> > > On hardware with Tx VLAN offload enabled, add the VLAN tag length to > the skb length before checking the Qbv maxSDU if Tx VLAN offload is > requested for the packet. Add 4 bytes for 802.1Q tag. This needs to say _why_. Please describe the problem that the current code suffers from. (e.g. the packet becomes too long for the queue to handle, which causes it to be dropped - which is my guess.) We shouldn't be guessing the reasons behind changes. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 12:44 ` Russell King (Oracle) @ 2025-10-18 2:06 ` G Thomas, Rohan 2025-10-23 10:58 ` Russell King (Oracle) 0 siblings, 1 reply; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-18 2:06 UTC (permalink / raw) To: Russell King (Oracle) Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi Russell, Thanks, I'll update the commit message. On 10/17/2025 6:14 PM, Russell King (Oracle) wrote: > On Fri, Oct 17, 2025 at 02:11:20PM +0800, Rohan G Thomas via B4 Relay wrote: >> From: Rohan G Thomas <rohan.g.thomas@altera.com> >> >> On hardware with Tx VLAN offload enabled, add the VLAN tag length to >> the skb length before checking the Qbv maxSDU if Tx VLAN offload is >> requested for the packet. Add 4 bytes for 802.1Q tag. > > This needs to say _why_. Please describe the problem that the current > code suffers from. (e.g. the packet becomes too long for the queue to > handle, which causes it to be dropped - which is my guess.) > > We shouldn't be guessing the reasons behind changes. > Queue maxSDU requirement of 802.1 Qbv standard requires mac to drop packets that exceeds maxSDU length and maxSDU doesn't include preamble, destination and source address, or FCS but includes ethernet type and VLAN header. On hardware with Tx VLAN offload enabled, VLAN header length is not included in the skb->len, when Tx VLAN offload is requested. This leads to incorrect length checks and allows transmission of oversized packets. Add the VLAN_HLEN to the skb->len before checking the Qbv maxSDU if Tx VLAN offload is requested for the packet. This patch ensures that the VLAN header length (`VLAN_HLEN`) is accounted for in the SDU length check when VLAN offload is requested. Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-18 2:06 ` G Thomas, Rohan @ 2025-10-23 10:58 ` Russell King (Oracle) 0 siblings, 0 replies; 19+ messages in thread From: Russell King (Oracle) @ 2025-10-23 10:58 UTC (permalink / raw) To: G Thomas, Rohan Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach On Sat, Oct 18, 2025 at 07:36:26AM +0530, G Thomas, Rohan wrote: > Hi Russell, > > Thanks, I'll update the commit message. > > On 10/17/2025 6:14 PM, Russell King (Oracle) wrote: > > On Fri, Oct 17, 2025 at 02:11:20PM +0800, Rohan G Thomas via B4 Relay wrote: > > > From: Rohan G Thomas <rohan.g.thomas@altera.com> > > > > > > On hardware with Tx VLAN offload enabled, add the VLAN tag length to > > > the skb length before checking the Qbv maxSDU if Tx VLAN offload is > > > requested for the packet. Add 4 bytes for 802.1Q tag. > > > > This needs to say _why_. Please describe the problem that the current > > code suffers from. (e.g. the packet becomes too long for the queue to > > handle, which causes it to be dropped - which is my guess.) > > > > We shouldn't be guessing the reasons behind changes. > > > > Queue maxSDU requirement of 802.1 Qbv standard requires mac to drop > packets that exceeds maxSDU length and maxSDU doesn't include preamble, > destination and source address, or FCS but includes ethernet type and VLAN > header. > > On hardware with Tx VLAN offload enabled, VLAN header length is not > included in the skb->len, when Tx VLAN offload is requested. This leads > to incorrect length checks and allows transmission of oversized packets. > Add the VLAN_HLEN to the skb->len before checking the Qbv maxSDU if Tx > VLAN offload is requested for the packet. > > This patch ensures that the VLAN header length (`VLAN_HLEN`) is > accounted for in the SDU length check when VLAN offload is requested. Please put that in the commit message, thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay 2025-10-17 7:36 ` G Thomas, Rohan 2025-10-17 12:44 ` Russell King (Oracle) @ 2025-10-17 17:16 ` kernel test robot 2025-10-27 9:03 ` G Thomas, Rohan 3 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2025-10-17 17:16 UTC (permalink / raw) To: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: llvm, oe-kbuild-all, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi Rohan, kernel test robot noticed the following build warnings: [auto build test WARNING on cb85ca4c0a349e246cd35161088aa3689ae5c580] url: https://github.com/intel-lab-lkp/linux/commits/Rohan-G-Thomas-via-B4-Relay/net-stmmac-vlan-Disable-802-1AD-tag-insertion-offload/20251017-144104 base: cb85ca4c0a349e246cd35161088aa3689ae5c580 patch link: https://lore.kernel.org/r/20251017-qbv-fixes-v3-2-d3a42e32646a%40altera.com patch subject: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU config: x86_64-kexec (https://download.01.org/0day-ci/archive/20251018/202510180039.zqD7oO26-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510180039.zqD7oO26-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/oe-kbuild-all/202510180039.zqD7oO26-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:4503:6: warning: variable 'sdu_len' set but not used [-Wunused-but-set-variable] 4503 | u32 sdu_len; | ^ 1 warning generated. vim +/sdu_len +4503 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 4478 4479 /** 4480 * stmmac_xmit - Tx entry point of the driver 4481 * @skb : the socket buffer 4482 * @dev : device pointer 4483 * Description : this is the tx entry point of the driver. 4484 * It programs the chain or the ring and supports oversized frames 4485 * and SG feature. 4486 */ 4487 static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) 4488 { 4489 unsigned int first_entry, tx_packets, enh_desc; 4490 struct stmmac_priv *priv = netdev_priv(dev); 4491 unsigned int nopaged_len = skb_headlen(skb); 4492 int i, csum_insertion = 0, is_jumbo = 0; 4493 u32 queue = skb_get_queue_mapping(skb); 4494 int nfrags = skb_shinfo(skb)->nr_frags; 4495 int gso = skb_shinfo(skb)->gso_type; 4496 struct stmmac_txq_stats *txq_stats; 4497 struct dma_edesc *tbs_desc = NULL; 4498 struct dma_desc *desc, *first; 4499 struct stmmac_tx_queue *tx_q; 4500 bool has_vlan, set_ic; 4501 int entry, first_tx; 4502 dma_addr_t des; > 4503 u32 sdu_len; 4504 4505 tx_q = &priv->dma_conf.tx_queue[queue]; 4506 txq_stats = &priv->xstats.txq_stats[queue]; 4507 first_tx = tx_q->cur_tx; 4508 4509 if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en) 4510 stmmac_stop_sw_lpi(priv); 4511 4512 /* Manage oversized TCP frames for GMAC4 device */ 4513 if (skb_is_gso(skb) && priv->tso) { 4514 if (gso & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) 4515 return stmmac_tso_xmit(skb, dev); 4516 if (priv->plat->has_gmac4 && (gso & SKB_GSO_UDP_L4)) 4517 return stmmac_tso_xmit(skb, dev); 4518 } 4519 4520 if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) { 4521 if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) { 4522 netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, 4523 queue)); 4524 /* This is a hard error, log it. */ 4525 netdev_err(priv->dev, 4526 "%s: Tx Ring full when queue awake\n", 4527 __func__); 4528 } 4529 return NETDEV_TX_BUSY; 4530 } 4531 4532 sdu_len = skb->len; 4533 /* Check if VLAN can be inserted by HW */ 4534 has_vlan = stmmac_vlan_insert(priv, skb, tx_q); 4535 if (has_vlan) 4536 sdu_len += VLAN_HLEN; 4537 4538 if (priv->est && priv->est->enable && 4539 priv->est->max_sdu[queue] && 4540 skb->len > priv->est->max_sdu[queue]){ 4541 priv->xstats.max_sdu_txq_drop[queue]++; 4542 goto max_sdu_err; 4543 } 4544 4545 entry = tx_q->cur_tx; 4546 first_entry = entry; 4547 WARN_ON(tx_q->tx_skbuff[first_entry]); 4548 4549 csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL); 4550 /* DWMAC IPs can be synthesized to support tx coe only for a few tx 4551 * queues. In that case, checksum offloading for those queues that don't 4552 * support tx coe needs to fallback to software checksum calculation. 4553 * 4554 * Packets that won't trigger the COE e.g. most DSA-tagged packets will 4555 * also have to be checksummed in software. 4556 */ 4557 if (csum_insertion && 4558 (priv->plat->tx_queues_cfg[queue].coe_unsupported || 4559 !stmmac_has_ip_ethertype(skb))) { 4560 if (unlikely(skb_checksum_help(skb))) 4561 goto dma_map_err; 4562 csum_insertion = !csum_insertion; 4563 } 4564 4565 if (likely(priv->extend_desc)) 4566 desc = (struct dma_desc *)(tx_q->dma_etx + entry); 4567 else if (tx_q->tbs & STMMAC_TBS_AVAIL) 4568 desc = &tx_q->dma_entx[entry].basic; 4569 else 4570 desc = tx_q->dma_tx + entry; 4571 4572 first = desc; 4573 4574 if (has_vlan) 4575 stmmac_set_desc_vlan(priv, first, STMMAC_VLAN_INSERT); 4576 4577 enh_desc = priv->plat->enh_desc; 4578 /* To program the descriptors according to the size of the frame */ 4579 if (enh_desc) 4580 is_jumbo = stmmac_is_jumbo_frm(priv, skb->len, enh_desc); 4581 4582 if (unlikely(is_jumbo)) { 4583 entry = stmmac_jumbo_frm(priv, tx_q, skb, csum_insertion); 4584 if (unlikely(entry < 0) && (entry != -EINVAL)) 4585 goto dma_map_err; 4586 } 4587 4588 for (i = 0; i < nfrags; i++) { 4589 const skb_frag_t *frag = &skb_shinfo(skb)->frags[i]; 4590 int len = skb_frag_size(frag); 4591 bool last_segment = (i == (nfrags - 1)); 4592 4593 entry = STMMAC_GET_ENTRY(entry, priv->dma_conf.dma_tx_size); 4594 WARN_ON(tx_q->tx_skbuff[entry]); 4595 4596 if (likely(priv->extend_desc)) 4597 desc = (struct dma_desc *)(tx_q->dma_etx + entry); 4598 else if (tx_q->tbs & STMMAC_TBS_AVAIL) 4599 desc = &tx_q->dma_entx[entry].basic; 4600 else 4601 desc = tx_q->dma_tx + entry; 4602 4603 des = skb_frag_dma_map(priv->device, frag, 0, len, 4604 DMA_TO_DEVICE); 4605 if (dma_mapping_error(priv->device, des)) 4606 goto dma_map_err; /* should reuse desc w/o issues */ 4607 4608 tx_q->tx_skbuff_dma[entry].buf = des; 4609 4610 stmmac_set_desc_addr(priv, desc, des); 4611 4612 tx_q->tx_skbuff_dma[entry].map_as_page = true; 4613 tx_q->tx_skbuff_dma[entry].len = len; 4614 tx_q->tx_skbuff_dma[entry].last_segment = last_segment; 4615 tx_q->tx_skbuff_dma[entry].buf_type = STMMAC_TXBUF_T_SKB; 4616 4617 /* Prepare the descriptor and set the own bit too */ 4618 stmmac_prepare_tx_desc(priv, desc, 0, len, csum_insertion, 4619 priv->mode, 1, last_segment, skb->len); 4620 } 4621 4622 /* Only the last descriptor gets to point to the skb. */ 4623 tx_q->tx_skbuff[entry] = skb; 4624 tx_q->tx_skbuff_dma[entry].buf_type = STMMAC_TXBUF_T_SKB; 4625 4626 /* According to the coalesce parameter the IC bit for the latest 4627 * segment is reset and the timer re-started to clean the tx status. 4628 * This approach takes care about the fragments: desc is the first 4629 * element in case of no SG. 4630 */ 4631 tx_packets = (entry + 1) - first_tx; 4632 tx_q->tx_count_frames += tx_packets; 4633 4634 if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) && priv->hwts_tx_en) 4635 set_ic = true; 4636 else if (!priv->tx_coal_frames[queue]) 4637 set_ic = false; 4638 else if (tx_packets > priv->tx_coal_frames[queue]) 4639 set_ic = true; 4640 else if ((tx_q->tx_count_frames % 4641 priv->tx_coal_frames[queue]) < tx_packets) 4642 set_ic = true; 4643 else 4644 set_ic = false; 4645 4646 if (set_ic) { 4647 if (likely(priv->extend_desc)) 4648 desc = &tx_q->dma_etx[entry].basic; 4649 else if (tx_q->tbs & STMMAC_TBS_AVAIL) 4650 desc = &tx_q->dma_entx[entry].basic; 4651 else 4652 desc = &tx_q->dma_tx[entry]; 4653 4654 tx_q->tx_count_frames = 0; 4655 stmmac_set_tx_ic(priv, desc); 4656 } 4657 4658 /* We've used all descriptors we need for this skb, however, 4659 * advance cur_tx so that it references a fresh descriptor. 4660 * ndo_start_xmit will fill this descriptor the next time it's 4661 * called and stmmac_tx_clean may clean up to this descriptor. 4662 */ 4663 entry = STMMAC_GET_ENTRY(entry, priv->dma_conf.dma_tx_size); 4664 tx_q->cur_tx = entry; 4665 4666 if (netif_msg_pktdata(priv)) { 4667 netdev_dbg(priv->dev, 4668 "%s: curr=%d dirty=%d f=%d, e=%d, first=%p, nfrags=%d", 4669 __func__, tx_q->cur_tx, tx_q->dirty_tx, first_entry, 4670 entry, first, nfrags); 4671 4672 netdev_dbg(priv->dev, ">>> frame to be transmitted: "); 4673 print_pkt(skb->data, skb->len); 4674 } 4675 4676 if (unlikely(stmmac_tx_avail(priv, queue) <= (MAX_SKB_FRAGS + 1))) { 4677 netif_dbg(priv, hw, priv->dev, "%s: stop transmitted packets\n", 4678 __func__); 4679 netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, queue)); 4680 } 4681 4682 u64_stats_update_begin(&txq_stats->q_syncp); 4683 u64_stats_add(&txq_stats->q.tx_bytes, skb->len); 4684 if (set_ic) 4685 u64_stats_inc(&txq_stats->q.tx_set_ic_bit); 4686 u64_stats_update_end(&txq_stats->q_syncp); 4687 4688 if (priv->sarc_type) 4689 stmmac_set_desc_sarc(priv, first, priv->sarc_type); 4690 4691 /* Ready to fill the first descriptor and set the OWN bit w/o any 4692 * problems because all the descriptors are actually ready to be 4693 * passed to the DMA engine. 4694 */ 4695 if (likely(!is_jumbo)) { 4696 bool last_segment = (nfrags == 0); 4697 4698 des = dma_map_single(priv->device, skb->data, 4699 nopaged_len, DMA_TO_DEVICE); 4700 if (dma_mapping_error(priv->device, des)) 4701 goto dma_map_err; 4702 4703 tx_q->tx_skbuff_dma[first_entry].buf = des; 4704 tx_q->tx_skbuff_dma[first_entry].buf_type = STMMAC_TXBUF_T_SKB; 4705 tx_q->tx_skbuff_dma[first_entry].map_as_page = false; 4706 4707 stmmac_set_desc_addr(priv, first, des); 4708 4709 tx_q->tx_skbuff_dma[first_entry].len = nopaged_len; 4710 tx_q->tx_skbuff_dma[first_entry].last_segment = last_segment; 4711 4712 if (unlikely((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) && 4713 priv->hwts_tx_en)) { 4714 /* declare that device is doing timestamping */ 4715 skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; 4716 stmmac_enable_tx_timestamp(priv, first); 4717 } 4718 4719 /* Prepare the first descriptor setting the OWN bit too */ 4720 stmmac_prepare_tx_desc(priv, first, 1, nopaged_len, 4721 csum_insertion, priv->mode, 0, last_segment, 4722 skb->len); 4723 } 4724 4725 if (tx_q->tbs & STMMAC_TBS_EN) { 4726 struct timespec64 ts = ns_to_timespec64(skb->tstamp); 4727 4728 tbs_desc = &tx_q->dma_entx[first_entry]; 4729 stmmac_set_desc_tbs(priv, tbs_desc, ts.tv_sec, ts.tv_nsec); 4730 } 4731 4732 stmmac_set_tx_owner(priv, first); 4733 4734 netdev_tx_sent_queue(netdev_get_tx_queue(dev, queue), skb->len); 4735 4736 stmmac_enable_dma_transmission(priv, priv->ioaddr, queue); 4737 skb_tx_timestamp(skb); 4738 stmmac_flush_tx_descriptors(priv, queue); 4739 stmmac_tx_timer_arm(priv, queue); 4740 4741 return NETDEV_TX_OK; 4742 4743 dma_map_err: 4744 netdev_err(priv->dev, "Tx DMA map failed\n"); 4745 max_sdu_err: 4746 dev_kfree_skb(skb); 4747 priv->xstats.tx_dropped++; 4748 return NETDEV_TX_OK; 4749 } 4750 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay ` (2 preceding siblings ...) 2025-10-17 17:16 ` kernel test robot @ 2025-10-27 9:03 ` G Thomas, Rohan 3 siblings, 0 replies; 19+ messages in thread From: G Thomas, Rohan @ 2025-10-27 9:03 UTC (permalink / raw) To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach Hi All, I've noticed one more issue with this commit. Need to drop the packet before inserting the context descriptor with VLAN. So I think I have to keep the max_sdu check in the original place itself, but add VLAN length if priv->dma_cap.vlins && skb_vlan_tag_present(skb) are true. Will change it accordingly in the next version. Apologies for the oversight in the initial patch. On 10/17/2025 11:41 AM, Rohan G Thomas via B4 Relay wrote: > From: Rohan G Thomas <rohan.g.thomas@altera.com> > > On hardware with Tx VLAN offload enabled, add the VLAN tag length to > the skb length before checking the Qbv maxSDU if Tx VLAN offload is > requested for the packet. Add 4 bytes for 802.1Q tag. > > Fixes: c5c3e1bfc9e0 ("net: stmmac: Offload queueMaxSDU from tc-taprio") > Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com> > Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index dedaaef3208bfadc105961029f79d0d26c3289d8..23bf4a3d324b7f8e8c3067ed4d47b436a89c97d3 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -4500,6 +4500,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > bool has_vlan, set_ic; > int entry, first_tx; > dma_addr_t des; > + u32 sdu_len; > > tx_q = &priv->dma_conf.tx_queue[queue]; > txq_stats = &priv->xstats.txq_stats[queue]; > @@ -4516,13 +4517,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > return stmmac_tso_xmit(skb, dev); > } > > - if (priv->est && priv->est->enable && > - priv->est->max_sdu[queue] && > - skb->len > priv->est->max_sdu[queue]){ > - priv->xstats.max_sdu_txq_drop[queue]++; > - goto max_sdu_err; > - } > - > if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) { > if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) { > netif_tx_stop_queue(netdev_get_tx_queue(priv->dev, > @@ -4535,8 +4529,18 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev) > return NETDEV_TX_BUSY; > } > > + sdu_len = skb->len; > /* Check if VLAN can be inserted by HW */ > has_vlan = stmmac_vlan_insert(priv, skb, tx_q); > + if (has_vlan) > + sdu_len += VLAN_HLEN; > + > + if (priv->est && priv->est->enable && > + priv->est->max_sdu[queue] && > + skb->len > priv->est->max_sdu[queue]){ > + priv->xstats.max_sdu_txq_drop[queue]++; > + goto max_sdu_err; > + } > > entry = tx_q->cur_tx; > first_entry = entry; > Best Regards, Rohan ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net v3 3/3] net: stmmac: est: Fix GCL bounds checks 2025-10-17 6:11 [PATCH net v3 0/3] net: stmmac: Fixes for stmmac Tx VLAN insert and EST Rohan G Thomas via B4 Relay 2025-10-17 6:11 ` [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload Rohan G Thomas via B4 Relay 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 ` Rohan G Thomas via B4 Relay 2 siblings, 0 replies; 19+ messages in thread From: Rohan G Thomas via B4 Relay @ 2025-10-17 6:11 UTC (permalink / raw) To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Jose Abreu, Rohan G Thomas, Boon Khai Ng Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, Rohan G Thomas, Matthew Gerlach From: Rohan G Thomas <rohan.g.thomas@altera.com> Fix the bounds checks for the hw supported maximum GCL entry count and gate interval time. Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API") Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c index 97e89a604abd7a01bb8e904c38f10716e0a911c1..3b4d4696afe96afe58e0c936429f51c22ae145be 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c @@ -981,7 +981,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, if (qopt->cmd == TAPRIO_CMD_DESTROY) goto disable; - if (qopt->num_entries >= dep) + if (qopt->num_entries > dep) return -EINVAL; if (!qopt->cycle_time) return -ERANGE; @@ -1012,7 +1012,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv, s64 delta_ns = qopt->entries[i].interval; u32 gates = qopt->entries[i].gate_mask; - if (delta_ns > GENMASK(wid, 0)) + if (delta_ns > GENMASK(wid - 1, 0)) return -ERANGE; if (gates > GENMASK(31 - wid, 0)) return -ERANGE; -- 2.43.7 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-10-27 9:04 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-17 6:11 [PATCH net v3 0/3] net: stmmac: Fixes for stmmac Tx VLAN insert and EST Rohan G Thomas via B4 Relay 2025-10-17 6:11 ` [PATCH net v3 1/3] net: stmmac: vlan: Disable 802.1AD tag insertion offload Rohan G Thomas via B4 Relay 2025-10-17 12:42 ` Russell King (Oracle) 2025-10-18 1:56 ` G Thomas, Rohan 2025-10-23 3:31 ` G Thomas, Rohan 2025-10-23 10:54 ` Russell King (Oracle) 2025-10-24 3:03 ` G Thomas, Rohan 2025-10-17 6:11 ` [PATCH net v3 2/3] net: stmmac: Consider Tx VLAN offload tag length for maxSDU Rohan G Thomas via B4 Relay 2025-10-17 7:36 ` G Thomas, Rohan 2025-10-17 12:21 ` Vadim Fedorenko 2025-10-18 1:50 ` G Thomas, Rohan 2025-10-23 10:59 ` Russell King (Oracle) 2025-10-23 16:03 ` G Thomas, Rohan 2025-10-17 12:44 ` Russell King (Oracle) 2025-10-18 2:06 ` G Thomas, Rohan 2025-10-23 10:58 ` Russell King (Oracle) 2025-10-17 17:16 ` kernel test robot 2025-10-27 9:03 ` G Thomas, Rohan 2025-10-17 6:11 ` [PATCH net v3 3/3] net: stmmac: est: Fix GCL bounds checks Rohan G Thomas via B4 Relay
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).