All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support
@ 2022-01-03 19:43 Aleksander Jan Bajkowski
  2022-01-04 12:40 ` patchwork-bot+netdevbpf
  2022-01-04 17:41 ` Eric Dumazet
  0 siblings, 2 replies; 4+ messages in thread
From: Aleksander Jan Bajkowski @ 2022-01-03 19:43 UTC (permalink / raw)
  To: hauke, davem, kuba, netdev, linux-kernel; +Cc: Aleksander Jan Bajkowski

This patch adds support for scatter gather DMA. DMA in PMAC splits
the packet into several buffers when the MTU on the CPU port is
less than the MTU of the switch. The first buffer starts at an
offset of NET_IP_ALIGN. In subsequent buffers, dma ignores the
offset. Thanks to this patch, the user can still connect to the
device in such a situation. For normal configurations, the patch
has no effect on performance.

Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 47 +++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 80bfaf2fec92..503fb99c5b90 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -27,6 +27,9 @@
 #define XRX200_DMA_TX		1
 #define XRX200_DMA_BURST_LEN	8
 
+#define XRX200_DMA_PACKET_COMPLETE	0
+#define XRX200_DMA_PACKET_IN_PROGRESS	1
+
 /* cpu port mac */
 #define PMAC_RX_IPG		0x0024
 #define PMAC_RX_IPG_MASK	0xf
@@ -62,6 +65,9 @@ struct xrx200_chan {
 	struct ltq_dma_channel dma;
 	struct sk_buff *skb[LTQ_DESC_NUM];
 
+	struct sk_buff *skb_head;
+	struct sk_buff *skb_tail;
+
 	struct xrx200_priv *priv;
 };
 
@@ -205,7 +211,8 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
 	struct xrx200_priv *priv = ch->priv;
 	struct ltq_dma_desc *desc = &ch->dma.desc_base[ch->dma.desc];
 	struct sk_buff *skb = ch->skb[ch->dma.desc];
-	int len = (desc->ctl & LTQ_DMA_SIZE_MASK);
+	u32 ctl = desc->ctl;
+	int len = (ctl & LTQ_DMA_SIZE_MASK);
 	struct net_device *net_dev = priv->net_dev;
 	int ret;
 
@@ -221,12 +228,36 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
 	}
 
 	skb_put(skb, len);
-	skb->protocol = eth_type_trans(skb, net_dev);
-	netif_receive_skb(skb);
-	net_dev->stats.rx_packets++;
-	net_dev->stats.rx_bytes += len;
 
-	return 0;
+	/* add buffers to skb via skb->frag_list */
+	if (ctl & LTQ_DMA_SOP) {
+		ch->skb_head = skb;
+		ch->skb_tail = skb;
+	} else if (ch->skb_head) {
+		if (ch->skb_head == ch->skb_tail)
+			skb_shinfo(ch->skb_tail)->frag_list = skb;
+		else
+			ch->skb_tail->next = skb;
+		ch->skb_tail = skb;
+		skb_reserve(ch->skb_tail, -NET_IP_ALIGN);
+		ch->skb_head->len += skb->len;
+		ch->skb_head->data_len += skb->len;
+		ch->skb_head->truesize += skb->truesize;
+	}
+
+	if (ctl & LTQ_DMA_EOP) {
+		ch->skb_head->protocol = eth_type_trans(ch->skb_head, net_dev);
+		netif_receive_skb(ch->skb_head);
+		net_dev->stats.rx_packets++;
+		net_dev->stats.rx_bytes += ch->skb_head->len;
+		ch->skb_head = NULL;
+		ch->skb_tail = NULL;
+		ret = XRX200_DMA_PACKET_COMPLETE;
+	} else {
+		ret = XRX200_DMA_PACKET_IN_PROGRESS;
+	}
+
+	return ret;
 }
 
 static int xrx200_poll_rx(struct napi_struct *napi, int budget)
@@ -241,7 +272,9 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
 
 		if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
 			ret = xrx200_hw_receive(ch);
-			if (ret)
+			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
+				continue;
+			if (ret != XRX200_DMA_PACKET_COMPLETE)
 				return ret;
 			rx++;
 		} else {
-- 
2.30.2


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

* Re: [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support
  2022-01-03 19:43 [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support Aleksander Jan Bajkowski
@ 2022-01-04 12:40 ` patchwork-bot+netdevbpf
  2022-01-04 17:41 ` Eric Dumazet
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-01-04 12:40 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski; +Cc: hauke, davem, kuba, netdev, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon,  3 Jan 2022 20:43:16 +0100 you wrote:
> This patch adds support for scatter gather DMA. DMA in PMAC splits
> the packet into several buffers when the MTU on the CPU port is
> less than the MTU of the switch. The first buffer starts at an
> offset of NET_IP_ALIGN. In subsequent buffers, dma ignores the
> offset. Thanks to this patch, the user can still connect to the
> device in such a situation. For normal configurations, the patch
> has no effect on performance.
> 
> [...]

Here is the summary with links:
  - [net-next] net: lantiq_xrx200: add ingress SG DMA support
    https://git.kernel.org/netdev/net-next/c/c3e6b2c35b34

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] 4+ messages in thread

* Re: [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support
  2022-01-03 19:43 [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support Aleksander Jan Bajkowski
  2022-01-04 12:40 ` patchwork-bot+netdevbpf
@ 2022-01-04 17:41 ` Eric Dumazet
  2022-01-08 13:27   ` Aleksander Bajkowski
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2022-01-04 17:41 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski, hauke, davem, kuba, netdev, linux-kernel


On 1/3/22 11:43, Aleksander Jan Bajkowski wrote:
> This patch adds support for scatter gather DMA. DMA in PMAC splits
> the packet into several buffers when the MTU on the CPU port is
> less than the MTU of the switch. The first buffer starts at an
> offset of NET_IP_ALIGN. In subsequent buffers, dma ignores the
> offset. Thanks to this patch, the user can still connect to the
> device in such a situation. For normal configurations, the patch
> has no effect on performance.
>
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
>   drivers/net/ethernet/lantiq_xrx200.c | 47 +++++++++++++++++++++++-----
>   1 file changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
> index 80bfaf2fec92..503fb99c5b90 100644
> --- a/drivers/net/ethernet/lantiq_xrx200.c
> +++ b/drivers/net/ethernet/lantiq_xrx200.c
> @@ -27,6 +27,9 @@
>   #define XRX200_DMA_TX		1
>   #define XRX200_DMA_BURST_LEN	8
>   
> +#define XRX200_DMA_PACKET_COMPLETE	0
> +#define XRX200_DMA_PACKET_IN_PROGRESS	1
> +
>   /* cpu port mac */
>   #define PMAC_RX_IPG		0x0024
>   #define PMAC_RX_IPG_MASK	0xf
> @@ -62,6 +65,9 @@ struct xrx200_chan {
>   	struct ltq_dma_channel dma;
>   	struct sk_buff *skb[LTQ_DESC_NUM];
>   
> +	struct sk_buff *skb_head;
> +	struct sk_buff *skb_tail;
> +
>   	struct xrx200_priv *priv;
>   };
>   
> @@ -205,7 +211,8 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>   	struct xrx200_priv *priv = ch->priv;
>   	struct ltq_dma_desc *desc = &ch->dma.desc_base[ch->dma.desc];
>   	struct sk_buff *skb = ch->skb[ch->dma.desc];
> -	int len = (desc->ctl & LTQ_DMA_SIZE_MASK);
> +	u32 ctl = desc->ctl;
> +	int len = (ctl & LTQ_DMA_SIZE_MASK);
>   	struct net_device *net_dev = priv->net_dev;
>   	int ret;
>   
> @@ -221,12 +228,36 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>   	}
>   
>   	skb_put(skb, len);
> -	skb->protocol = eth_type_trans(skb, net_dev);
> -	netif_receive_skb(skb);
> -	net_dev->stats.rx_packets++;
> -	net_dev->stats.rx_bytes += len;
>   
> -	return 0;
> +	/* add buffers to skb via skb->frag_list */
> +	if (ctl & LTQ_DMA_SOP) {
> +		ch->skb_head = skb;
> +		ch->skb_tail = skb;
> +	} else if (ch->skb_head) {
> +		if (ch->skb_head == ch->skb_tail)
> +			skb_shinfo(ch->skb_tail)->frag_list = skb;
> +		else
> +			ch->skb_tail->next = skb;
> +		ch->skb_tail = skb;
> +		skb_reserve(ch->skb_tail, -NET_IP_ALIGN);
> +		ch->skb_head->len += skb->len;
> +		ch->skb_head->data_len += skb->len;
> +		ch->skb_head->truesize += skb->truesize;
> +	}
> +
> +	if (ctl & LTQ_DMA_EOP) {
> +		ch->skb_head->protocol = eth_type_trans(ch->skb_head, net_dev);
> +		netif_receive_skb(ch->skb_head);
> +		net_dev->stats.rx_packets++;
> +		net_dev->stats.rx_bytes += ch->skb_head->len;


Use after free alert.

Please add/test the following fix.

(It is illegal to deref skb after netif_receive_skb())


diff --git a/drivers/net/ethernet/lantiq_xrx200.c 
b/drivers/net/ethernet/lantiq_xrx200.c
index 503fb99c5b90..bf7e3c7910d1 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -247,9 +247,9 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)

         if (ctl & LTQ_DMA_EOP) {
                 ch->skb_head->protocol = eth_type_trans(ch->skb_head, 
net_dev);
-               netif_receive_skb(ch->skb_head);
                 net_dev->stats.rx_packets++;
                 net_dev->stats.rx_bytes += ch->skb_head->len;
+               netif_receive_skb(ch->skb_head);
                 ch->skb_head = NULL;
                 ch->skb_tail = NULL;
                 ret = XRX200_DMA_PACKET_COMPLETE;



> +		ch->skb_head = NULL;
> +		ch->skb_tail = NULL;
> +		ret = XRX200_DMA_PACKET_COMPLETE;
> +	} else {
> +		ret = XRX200_DMA_PACKET_IN_PROGRESS;
> +	}
> +
> +	return ret;
>   }
>   
>   static int xrx200_poll_rx(struct napi_struct *napi, int budget)
> @@ -241,7 +272,9 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
>   
>   		if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
>   			ret = xrx200_hw_receive(ch);
> -			if (ret)
> +			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
> +				continue;
> +			if (ret != XRX200_DMA_PACKET_COMPLETE)
>   				return ret;
>   			rx++;
>   		} else {

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

* Re: [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support
  2022-01-04 17:41 ` Eric Dumazet
@ 2022-01-08 13:27   ` Aleksander Bajkowski
  0 siblings, 0 replies; 4+ messages in thread
From: Aleksander Bajkowski @ 2022-01-08 13:27 UTC (permalink / raw)
  To: Eric Dumazet, hauke, davem, kuba, netdev, linux-kernel

Hi Eric,

On 1/4/22 18:41, Eric Dumazet wrote:
> 
> On 1/3/22 11:43, Aleksander Jan Bajkowski wrote:
>> This patch adds support for scatter gather DMA. DMA in PMAC splits
>> the packet into several buffers when the MTU on the CPU port is
>> less than the MTU of the switch. The first buffer starts at an
>> offset of NET_IP_ALIGN. In subsequent buffers, dma ignores the
>> offset. Thanks to this patch, the user can still connect to the
>> device in such a situation. For normal configurations, the patch
>> has no effect on performance.
>>
>> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
>> ---
>>   drivers/net/ethernet/lantiq_xrx200.c | 47 +++++++++++++++++++++++-----
>>   1 file changed, 40 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
>> index 80bfaf2fec92..503fb99c5b90 100644
>> --- a/drivers/net/ethernet/lantiq_xrx200.c
>> +++ b/drivers/net/ethernet/lantiq_xrx200.c
>> @@ -27,6 +27,9 @@
>>   #define XRX200_DMA_TX        1
>>   #define XRX200_DMA_BURST_LEN    8
>>   +#define XRX200_DMA_PACKET_COMPLETE    0
>> +#define XRX200_DMA_PACKET_IN_PROGRESS    1
>> +
>>   /* cpu port mac */
>>   #define PMAC_RX_IPG        0x0024
>>   #define PMAC_RX_IPG_MASK    0xf
>> @@ -62,6 +65,9 @@ struct xrx200_chan {
>>       struct ltq_dma_channel dma;
>>       struct sk_buff *skb[LTQ_DESC_NUM];
>>   +    struct sk_buff *skb_head;
>> +    struct sk_buff *skb_tail;
>> +
>>       struct xrx200_priv *priv;
>>   };
>>   @@ -205,7 +211,8 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>>       struct xrx200_priv *priv = ch->priv;
>>       struct ltq_dma_desc *desc = &ch->dma.desc_base[ch->dma.desc];
>>       struct sk_buff *skb = ch->skb[ch->dma.desc];
>> -    int len = (desc->ctl & LTQ_DMA_SIZE_MASK);
>> +    u32 ctl = desc->ctl;
>> +    int len = (ctl & LTQ_DMA_SIZE_MASK);
>>       struct net_device *net_dev = priv->net_dev;
>>       int ret;
>>   @@ -221,12 +228,36 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>>       }
>>         skb_put(skb, len);
>> -    skb->protocol = eth_type_trans(skb, net_dev);
>> -    netif_receive_skb(skb);
>> -    net_dev->stats.rx_packets++;
>> -    net_dev->stats.rx_bytes += len;
>>   -    return 0;
>> +    /* add buffers to skb via skb->frag_list */
>> +    if (ctl & LTQ_DMA_SOP) {
>> +        ch->skb_head = skb;
>> +        ch->skb_tail = skb;
>> +    } else if (ch->skb_head) {
>> +        if (ch->skb_head == ch->skb_tail)
>> +            skb_shinfo(ch->skb_tail)->frag_list = skb;
>> +        else
>> +            ch->skb_tail->next = skb;
>> +        ch->skb_tail = skb;
>> +        skb_reserve(ch->skb_tail, -NET_IP_ALIGN);
>> +        ch->skb_head->len += skb->len;
>> +        ch->skb_head->data_len += skb->len;
>> +        ch->skb_head->truesize += skb->truesize;
>> +    }
>> +
>> +    if (ctl & LTQ_DMA_EOP) {
>> +        ch->skb_head->protocol = eth_type_trans(ch->skb_head, net_dev);
>> +        netif_receive_skb(ch->skb_head);
>> +        net_dev->stats.rx_packets++;
>> +        net_dev->stats.rx_bytes += ch->skb_head->len;
> 
> 
> Use after free alert.
> 
> Please add/test the following fix.
> 
> (It is illegal to deref skb after netif_receive_skb())
> 
> 
> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
> index 503fb99c5b90..bf7e3c7910d1 100644
> --- a/drivers/net/ethernet/lantiq_xrx200.c
> +++ b/drivers/net/ethernet/lantiq_xrx200.c
> @@ -247,9 +247,9 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
> 
>         if (ctl & LTQ_DMA_EOP) {
>                 ch->skb_head->protocol = eth_type_trans(ch->skb_head, net_dev);
> -               netif_receive_skb(ch->skb_head);
>                 net_dev->stats.rx_packets++;
>                 net_dev->stats.rx_bytes += ch->skb_head->len;
> +               netif_receive_skb(ch->skb_head);
>                 ch->skb_head = NULL;
>                 ch->skb_tail = NULL;
>                 ret = XRX200_DMA_PACKET_COMPLETE;
> 
> 
>


Thanks for spot this bug. I tested this patch and it works
ok. I will sent this patch it soon. 


>> +        ch->skb_head = NULL;
>> +        ch->skb_tail = NULL;
>> +        ret = XRX200_DMA_PACKET_COMPLETE;
>> +    } else {
>> +        ret = XRX200_DMA_PACKET_IN_PROGRESS;
>> +    }
>> +
>> +    return ret;
>>   }
>>     static int xrx200_poll_rx(struct napi_struct *napi, int budget)
>> @@ -241,7 +272,9 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
>>             if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
>>               ret = xrx200_hw_receive(ch);
>> -            if (ret)
>> +            if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
>> +                continue;
>> +            if (ret != XRX200_DMA_PACKET_COMPLETE)
>>                   return ret;
>>               rx++;
>>           } else {

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

end of thread, other threads:[~2022-01-08 13:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 19:43 [PATCH net-next] net: lantiq_xrx200: add ingress SG DMA support Aleksander Jan Bajkowski
2022-01-04 12:40 ` patchwork-bot+netdevbpf
2022-01-04 17:41 ` Eric Dumazet
2022-01-08 13:27   ` Aleksander Bajkowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.