Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases
@ 2021-08-05 14:55 Grygorii Strashko
  2021-08-05 18:43 ` Grygorii Strashko
  2021-08-09  9:20 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Grygorii Strashko @ 2021-08-05 14:55 UTC (permalink / raw)
  To: David S. Miller, netdev, Jakub Kicinski, Ben Hutchings
  Cc: linux-kernel, Vignesh Raghavendra, linux-omap, Lokesh Vutla,
	Grygorii Strashko, stable

The CPSW switchdev driver inherited fix from commit 9421c9015047 ("net:
ethernet: ti: cpsw: fix min eth packet size") which changes min TX packet
size to 64bytes (VLAN_ETH_ZLEN, excluding ETH_FCS). It was done to fix HW
packed drop issue when packets are sent from Host to the port with PVID and
un-tagging enabled. Unfortunately this breaks some other non-switch
specific use-cases, like:
- [1] CPSW port as DSA CPU port with DSA-tag applied at the end of the
packet
- [2] Some industrial protocols, which expects min TX packet size 60Bytes
(excluding FCS).

Fix it by configuring min TX packet size depending on driver mode
 - 60Bytes (ETH_ZLEN) for multi mac (dual-mac) mode
 - 64Bytes (VLAN_ETH_ZLEN) for switch mode
and update it during driver mode change and annotate with
READ_ONCE()/WRITE_ONCE() as it can be read by napi while writing.

[1] https://lore.kernel.org/netdev/20210531124051.GA15218@cephalopod/
[2] https://e2e.ti.com/support/arm/sitara_arm/f/791/t/701669


Cc: stable@vger.kernel.org
Fixes: ed3525eda4c4 ("net: ethernet: ti: introduce cpsw switchdev based driver part 1 - dual-emac")
Reported-by: Ben Hutchings <ben.hutchings@essensium.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---

Changes in v2:
- use skb_put_padto
- update description
- annotate tx_packet_min with READ_ONCE()/WRITE_ONCE()

I'm not going to add additional changes in cpdma configuration interface and, 
instead, will send patches to convert all cpdma users to use skb_put_padto() and
drop frames padding from cpdma.

v1: https://patchwork.kernel.org/project/netdevbpf/patch/20210611132732.10690-1-grygorii.strashko@ti.com/

 drivers/net/ethernet/ti/cpsw_new.c  | 7 +++++--
 drivers/net/ethernet/ti/cpsw_priv.h | 4 +++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index ae167223e87f..d904f4ca4b37 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -921,7 +921,7 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 	struct cpdma_chan *txch;
 	int ret, q_idx;
 
-	if (skb_padto(skb, CPSW_MIN_PACKET_SIZE)) {
+	if (skb_put_padto(skb, READ_ONCE(priv->tx_packet_min))) {
 		cpsw_err(priv, tx_err, "packet pad failed\n");
 		ndev->stats.tx_dropped++;
 		return NET_XMIT_DROP;
@@ -1101,7 +1101,7 @@ static int cpsw_ndo_xdp_xmit(struct net_device *ndev, int n,
 
 	for (i = 0; i < n; i++) {
 		xdpf = frames[i];
-		if (xdpf->len < CPSW_MIN_PACKET_SIZE)
+		if (xdpf->len < READ_ONCE(priv->tx_packet_min))
 			break;
 
 		if (cpsw_xdp_tx_frame(priv, xdpf, NULL, priv->emac_port))
@@ -1390,6 +1390,7 @@ static int cpsw_create_ports(struct cpsw_common *cpsw)
 		priv->dev  = dev;
 		priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
 		priv->emac_port = i + 1;
+		priv->tx_packet_min = CPSW_MIN_PACKET_SIZE;
 
 		if (is_valid_ether_addr(slave_data->mac_addr)) {
 			ether_addr_copy(priv->mac_addr, slave_data->mac_addr);
@@ -1698,6 +1699,7 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
 
 			priv = netdev_priv(sl_ndev);
 			slave->port_vlan = vlan;
+			WRITE_ONCE(priv->tx_packet_min, CPSW_MIN_PACKET_SIZE_VLAN);
 			if (netif_running(sl_ndev))
 				cpsw_port_add_switch_def_ale_entries(priv,
 								     slave);
@@ -1726,6 +1728,7 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
 
 			priv = netdev_priv(slave->ndev);
 			slave->port_vlan = slave->data->dual_emac_res_vlan;
+			WRITE_ONCE(priv->tx_packet_min, CPSW_MIN_PACKET_SIZE);
 			cpsw_port_add_dual_emac_def_ale_entries(priv, slave);
 		}
 
diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
index a323bea54faa..2951fb7b9dae 100644
--- a/drivers/net/ethernet/ti/cpsw_priv.h
+++ b/drivers/net/ethernet/ti/cpsw_priv.h
@@ -89,7 +89,8 @@ do {								\
 
 #define CPSW_POLL_WEIGHT	64
 #define CPSW_RX_VLAN_ENCAP_HDR_SIZE		4
-#define CPSW_MIN_PACKET_SIZE	(VLAN_ETH_ZLEN)
+#define CPSW_MIN_PACKET_SIZE_VLAN	(VLAN_ETH_ZLEN)
+#define CPSW_MIN_PACKET_SIZE	(ETH_ZLEN)
 #define CPSW_MAX_PACKET_SIZE	(VLAN_ETH_FRAME_LEN +\
 				 ETH_FCS_LEN +\
 				 CPSW_RX_VLAN_ENCAP_HDR_SIZE)
@@ -380,6 +381,7 @@ struct cpsw_priv {
 	u32 emac_port;
 	struct cpsw_common *cpsw;
 	int offload_fwd_mark;
+	u32 tx_packet_min;
 };
 
 #define ndev_to_cpsw(ndev) (((struct cpsw_priv *)netdev_priv(ndev))->cpsw)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases
  2021-08-05 14:55 [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases Grygorii Strashko
@ 2021-08-05 18:43 ` Grygorii Strashko
  2021-08-09  9:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Grygorii Strashko @ 2021-08-05 18:43 UTC (permalink / raw)
  To: David S. Miller, netdev, Jakub Kicinski, Ben Hutchings
  Cc: linux-kernel, Vignesh Raghavendra, linux-omap, Lokesh Vutla,
	stable, Ilias Apalodimas


On 05/08/2021 17:55, Grygorii Strashko wrote:
> The CPSW switchdev driver inherited fix from commit 9421c9015047 ("net:
> ethernet: ti: cpsw: fix min eth packet size") which changes min TX packet
> size to 64bytes (VLAN_ETH_ZLEN, excluding ETH_FCS). It was done to fix HW
> packed drop issue when packets are sent from Host to the port with PVID and
> un-tagging enabled. Unfortunately this breaks some other non-switch
> specific use-cases, like:
> - [1] CPSW port as DSA CPU port with DSA-tag applied at the end of the
> packet
> - [2] Some industrial protocols, which expects min TX packet size 60Bytes
> (excluding FCS).
> 
> Fix it by configuring min TX packet size depending on driver mode
>   - 60Bytes (ETH_ZLEN) for multi mac (dual-mac) mode
>   - 64Bytes (VLAN_ETH_ZLEN) for switch mode
> and update it during driver mode change and annotate with
> READ_ONCE()/WRITE_ONCE() as it can be read by napi while writing.
> 
> [1] https://lore.kernel.org/netdev/20210531124051.GA15218@cephalopod/
> [2] https://e2e.ti.com/support/arm/sitara_arm/f/791/t/701669
> 
> 
> Cc: stable@vger.kernel.org
> Fixes: ed3525eda4c4 ("net: ethernet: ti: introduce cpsw switchdev based driver part 1 - dual-emac")
> Reported-by: Ben Hutchings <ben.hutchings@essensium.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> 
> Changes in v2:
> - use skb_put_padto
> - update description
> - annotate tx_packet_min with READ_ONCE()/WRITE_ONCE()
> 
> I'm not going to add additional changes in cpdma configuration interface and,
> instead, will send patches to convert all cpdma users to use skb_put_padto() and
> drop frames padding from cpdma.
> 
> v1: https://patchwork.kernel.org/project/netdevbpf/patch/20210611132732.10690-1-grygorii.strashko@ti.com/
> 
>   drivers/net/ethernet/ti/cpsw_new.c  | 7 +++++--
>   drivers/net/ethernet/ti/cpsw_priv.h | 4 +++-
>   2 files changed, 8 insertions(+), 3 deletions(-)
> 

Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>

> diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
> index ae167223e87f..d904f4ca4b37 100644
> --- a/drivers/net/ethernet/ti/cpsw_new.c
> +++ b/drivers/net/ethernet/ti/cpsw_new.c
> @@ -921,7 +921,7 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
>   	struct cpdma_chan *txch;
>   	int ret, q_idx;
>   
> -	if (skb_padto(skb, CPSW_MIN_PACKET_SIZE)) {
> +	if (skb_put_padto(skb, READ_ONCE(priv->tx_packet_min))) {
>   		cpsw_err(priv, tx_err, "packet pad failed\n");
>   		ndev->stats.tx_dropped++;
>   		return NET_XMIT_DROP;
> @@ -1101,7 +1101,7 @@ static int cpsw_ndo_xdp_xmit(struct net_device *ndev, int n,
>   
>   	for (i = 0; i < n; i++) {
>   		xdpf = frames[i];
> -		if (xdpf->len < CPSW_MIN_PACKET_SIZE)
> +		if (xdpf->len < READ_ONCE(priv->tx_packet_min))
>   			break;
>   
>   		if (cpsw_xdp_tx_frame(priv, xdpf, NULL, priv->emac_port))
> @@ -1390,6 +1390,7 @@ static int cpsw_create_ports(struct cpsw_common *cpsw)
>   		priv->dev  = dev;
>   		priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
>   		priv->emac_port = i + 1;
> +		priv->tx_packet_min = CPSW_MIN_PACKET_SIZE;
>   
>   		if (is_valid_ether_addr(slave_data->mac_addr)) {
>   			ether_addr_copy(priv->mac_addr, slave_data->mac_addr);
> @@ -1698,6 +1699,7 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
>   
>   			priv = netdev_priv(sl_ndev);
>   			slave->port_vlan = vlan;
> +			WRITE_ONCE(priv->tx_packet_min, CPSW_MIN_PACKET_SIZE_VLAN);
>   			if (netif_running(sl_ndev))
>   				cpsw_port_add_switch_def_ale_entries(priv,
>   								     slave);
> @@ -1726,6 +1728,7 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
>   
>   			priv = netdev_priv(slave->ndev);
>   			slave->port_vlan = slave->data->dual_emac_res_vlan;
> +			WRITE_ONCE(priv->tx_packet_min, CPSW_MIN_PACKET_SIZE);
>   			cpsw_port_add_dual_emac_def_ale_entries(priv, slave);
>   		}
>   
> diff --git a/drivers/net/ethernet/ti/cpsw_priv.h b/drivers/net/ethernet/ti/cpsw_priv.h
> index a323bea54faa..2951fb7b9dae 100644
> --- a/drivers/net/ethernet/ti/cpsw_priv.h
> +++ b/drivers/net/ethernet/ti/cpsw_priv.h
> @@ -89,7 +89,8 @@ do {								\
>   
>   #define CPSW_POLL_WEIGHT	64
>   #define CPSW_RX_VLAN_ENCAP_HDR_SIZE		4
> -#define CPSW_MIN_PACKET_SIZE	(VLAN_ETH_ZLEN)
> +#define CPSW_MIN_PACKET_SIZE_VLAN	(VLAN_ETH_ZLEN)
> +#define CPSW_MIN_PACKET_SIZE	(ETH_ZLEN)
>   #define CPSW_MAX_PACKET_SIZE	(VLAN_ETH_FRAME_LEN +\
>   				 ETH_FCS_LEN +\
>   				 CPSW_RX_VLAN_ENCAP_HDR_SIZE)
> @@ -380,6 +381,7 @@ struct cpsw_priv {
>   	u32 emac_port;
>   	struct cpsw_common *cpsw;
>   	int offload_fwd_mark;
> +	u32 tx_packet_min;
>   };
>   
>   #define ndev_to_cpsw(ndev) (((struct cpsw_priv *)netdev_priv(ndev))->cpsw)
> 

-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases
  2021-08-05 14:55 [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases Grygorii Strashko
  2021-08-05 18:43 ` Grygorii Strashko
@ 2021-08-09  9:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-08-09  9:20 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: davem, netdev, kuba, ben.hutchings, linux-kernel, vigneshr,
	linux-omap, lokeshvutla, stable

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Thu, 5 Aug 2021 17:55:11 +0300 you wrote:
> The CPSW switchdev driver inherited fix from commit 9421c9015047 ("net:
> ethernet: ti: cpsw: fix min eth packet size") which changes min TX packet
> size to 64bytes (VLAN_ETH_ZLEN, excluding ETH_FCS). It was done to fix HW
> packed drop issue when packets are sent from Host to the port with PVID and
> un-tagging enabled. Unfortunately this breaks some other non-switch
> specific use-cases, like:
> - [1] CPSW port as DSA CPU port with DSA-tag applied at the end of the
> packet
> - [2] Some industrial protocols, which expects min TX packet size 60Bytes
> (excluding FCS).
> 
> [...]

Here is the summary with links:
  - [net,v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases
    https://git.kernel.org/netdev/net/c/acc68b8d2a11

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-09  9:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 14:55 [PATCH net v2] net: ethernet: ti: cpsw: fix min eth packet size for non-switch use-cases Grygorii Strashko
2021-08-05 18:43 ` Grygorii Strashko
2021-08-09  9:20 ` patchwork-bot+netdevbpf

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