[RESEND PATCH net-next 1/2] net: mhi: Allow decoupled MTU/MRU

[RESEND PATCH net-next 1/2] net: mhi: Allow decoupled MTU/MRU

[Loic Poulain]

If a maximum receive unit (MRU) size is specified, use it for RX
buffers allocation instead of the MTU.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/mhi/mhi.h | 1 +
 drivers/net/mhi/net.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mhi/mhi.h b/drivers/net/mhi/mhi.h
index 12e7407..1d0c499 100644
--- a/drivers/net/mhi/mhi.h
+++ b/drivers/net/mhi/mhi.h
@@ -29,6 +29,7 @@ struct mhi_net_dev {
 	struct mhi_net_stats stats;
 	u32 rx_queue_sz;
 	int msg_enable;
+	unsigned int mru;
 };
 
 struct mhi_net_proto {
diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index f599608..5ec7a29 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c
@@ -265,10 +265,12 @@ static void mhi_net_rx_refill_work(struct work_struct *work)
 						      rx_refill.work);
 	struct net_device *ndev = mhi_netdev->ndev;
 	struct mhi_device *mdev = mhi_netdev->mdev;
-	int size = READ_ONCE(ndev->mtu);
 	struct sk_buff *skb;
+	unsigned int size;
 	int err;
 
+	size = mhi_netdev->mru ? mhi_netdev->mru : READ_ONCE(ndev->mtu);
+
 	while (!mhi_queue_is_full(mdev, DMA_FROM_DEVICE)) {
 		skb = netdev_alloc_skb(ndev, size);
 		if (unlikely(!skb))
-- 
2.7.4

[RESEND PATCH net-next 2/2] net: mhi: proto_mbim: Adjust MTU and MRU

[Loic Poulain]

MBIM protocol makes the interface asymmetric, ingress data received
from MHI is MBIM protocol, that can contain multiple aggregated IP
packets, while egress data received from network stack is IP protocol.

Set a default MTU to 1500 (usual network MTU for WWAN), and MRU to 32K
which is the default size of MBIM-over-MHI packets.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/mhi/proto_mbim.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/mhi/proto_mbim.c b/drivers/net/mhi/proto_mbim.c
index 75b5484..29d8577 100644
--- a/drivers/net/mhi/proto_mbim.c
+++ b/drivers/net/mhi/proto_mbim.c
@@ -26,6 +26,9 @@
 
 #define MBIM_NDP16_SIGN_MASK 0x00ffffff
 
+#define MHI_MBIM_DEFAULT_MRU 32768
+#define MHI_MBIM_DEFAULT_MTU 1500
+
 struct mbim_context {
 	u16 rx_seq;
 	u16 tx_seq;
@@ -282,6 +285,8 @@ static int mbim_init(struct mhi_net_dev *mhi_netdev)
 		return -ENOMEM;
 
 	ndev->needed_headroom = sizeof(struct mbim_tx_hdr);
+	ndev->mtu = MHI_MBIM_DEFAULT_MTU;
+	mhi_netdev->mru = MHI_MBIM_DEFAULT_MRU;
 
 	return 0;
 }
-- 
2.7.4

Re: [RESEND PATCH net-next 1/2] net: mhi: Allow decoupled MTU/MRU

[Jakub Kicinski]

On Tue, 23 Mar 2021 15:45:06 +0100 Loic Poulain wrote:
> If a maximum receive unit (MRU) size is specified, use it for RX
> buffers allocation instead of the MTU.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

I don't think this patch represents a logical change. You should merge
your patches into one.

> diff --git a/drivers/net/mhi/mhi.h b/drivers/net/mhi/mhi.h
> index 12e7407..1d0c499 100644
> --- a/drivers/net/mhi/mhi.h
> +++ b/drivers/net/mhi/mhi.h
> @@ -29,6 +29,7 @@ struct mhi_net_dev {
>  	struct mhi_net_stats stats;
>  	u32 rx_queue_sz;
>  	int msg_enable;
> +	unsigned int mru;
>  };
>  
>  struct mhi_net_proto {
> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
> index f599608..5ec7a29 100644
> --- a/drivers/net/mhi/net.c
> +++ b/drivers/net/mhi/net.c
> @@ -265,10 +265,12 @@ static void mhi_net_rx_refill_work(struct work_struct *work)
>  						      rx_refill.work);
>  	struct net_device *ndev = mhi_netdev->ndev;
>  	struct mhi_device *mdev = mhi_netdev->mdev;
> -	int size = READ_ONCE(ndev->mtu);
>  	struct sk_buff *skb;
> +	unsigned int size;
>  	int err;
>  
> +	size = mhi_netdev->mru ? mhi_netdev->mru : READ_ONCE(ndev->mtu);
> +
>  	while (!mhi_queue_is_full(mdev, DMA_FROM_DEVICE)) {
>  		skb = netdev_alloc_skb(ndev, size);
>  		if (unlikely(!skb))

Re: [RESEND PATCH net-next 2/2] net: mhi: proto_mbim: Adjust MTU and MRU

[Jakub Kicinski]

On Tue, 23 Mar 2021 15:45:07 +0100 Loic Poulain wrote:
> MBIM protocol makes the interface asymmetric, ingress data received
> from MHI is MBIM protocol, that can contain multiple aggregated IP
> packets, while egress data received from network stack is IP protocol.
> 
> Set a default MTU to 1500 (usual network MTU for WWAN), and MRU to 32K
> which is the default size of MBIM-over-MHI packets.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/net/mhi/proto_mbim.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/mhi/proto_mbim.c b/drivers/net/mhi/proto_mbim.c
> index 75b5484..29d8577 100644
> --- a/drivers/net/mhi/proto_mbim.c
> +++ b/drivers/net/mhi/proto_mbim.c
> @@ -26,6 +26,9 @@
>  
>  #define MBIM_NDP16_SIGN_MASK 0x00ffffff
>  
> +#define MHI_MBIM_DEFAULT_MRU 32768
> +#define MHI_MBIM_DEFAULT_MTU 1500
> +
>  struct mbim_context {
>  	u16 rx_seq;
>  	u16 tx_seq;
> @@ -282,6 +285,8 @@ static int mbim_init(struct mhi_net_dev *mhi_netdev)
>  		return -ENOMEM;
>  
>  	ndev->needed_headroom = sizeof(struct mbim_tx_hdr);
> +	ndev->mtu = MHI_MBIM_DEFAULT_MTU;
> +	mhi_netdev->mru = MHI_MBIM_DEFAULT_MRU;
>  
>  	return 0;
>  }

32k + skb overhead will result in rather large contiguous allocation.
Using ~3.5k buffers (basically a page - paddings and skb_shinfo) should
be much more resilient, and still very efficient.

This sort of 32k buffer thing is common for USB, but I thought MHI is
over PCI so there should be no bus considerations once we're above 1k.