[PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[Subash Abhinov Kasiviswanathan]

For deaggregation, the real device receives a large linear skb and
passes it on to rmnet. rmnet creates new skbs from this large frame.

If the real device supports recycling, it does not need to allocate
the large skbs during packet reception and can instead reuse them.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 29842cc..7869fcf 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -108,7 +108,10 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 		while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
 			__rmnet_map_ingress_handler(skbn, port);
 
-		consume_skb(skb);
+		if (skb->destructor)
+			skb->destructor(skb);
+		else
+			consume_skb(skb);
 	} else {
 		__rmnet_map_ingress_handler(skb, port);
 	}
-- 
1.9.1

Re: [PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[Alexander Duyck]

On Fri, Oct 27, 2017 at 1:30 PM, Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org> wrote:
> For deaggregation, the real device receives a large linear skb and
> passes it on to rmnet. rmnet creates new skbs from this large frame.
>
> If the real device supports recycling, it does not need to allocate
> the large skbs during packet reception and can instead reuse them.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> ---
>  drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 29842cc..7869fcf 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> @@ -108,7 +108,10 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
>                 while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
>                         __rmnet_map_ingress_handler(skbn, port);
>
> -               consume_skb(skb);
> +               if (skb->destructor)
> +                       skb->destructor(skb);
> +               else
> +                       consume_skb(skb);
>         } else {
>                 __rmnet_map_ingress_handler(skb, port);
>         }

This doesn't make sense to me, maybe I am missing something. What
"real device" is setting the skb->destructor() and doing it to somehow
handle recycling? The only driver I can find that is setting
skb->desctructor() is the Chelsio drivers, and they appear to be using
it to just clean-up DMA mappings in their transmit path.

Thanks.

- Alex

Re: [PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[Subash Abhinov Kasiviswanathan]

> This doesn't make sense to me, maybe I am missing something. What
> "real device" is setting the skb->destructor() and doing it to somehow
> handle recycling? The only driver I can find that is setting
> skb->desctructor() is the Chelsio drivers, and they appear to be using
> it to just clean-up DMA mappings in their transmit path.
> 
> Thanks.
> 
> - Alex

Hi Alex

The real device will have a pool of skbs and mark them as unused on
initialization. It will use these skbs to copy data from hardware and
pass them onto rmnet after marking them as used. Once rmnet is done
processing, the callback of the real device set in the skb_destructor
is invoked and the skb is set as unused and can be used again for
copying data.

There are no drivers setting the skb->destructor() in receive path
upstream as of now but the plan is to get them there.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[Alexander Duyck]

On Fri, Oct 27, 2017 at 3:22 PM, Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org> wrote:
>> This doesn't make sense to me, maybe I am missing something. What
>> "real device" is setting the skb->destructor() and doing it to somehow
>> handle recycling? The only driver I can find that is setting
>> skb->desctructor() is the Chelsio drivers, and they appear to be using
>> it to just clean-up DMA mappings in their transmit path.
>>
>> Thanks.
>>
>> - Alex
>
>
> Hi Alex
>
> The real device will have a pool of skbs and mark them as unused on
> initialization. It will use these skbs to copy data from hardware and
> pass them onto rmnet after marking them as used. Once rmnet is done
> processing, the callback of the real device set in the skb_destructor
> is invoked and the skb is set as unused and can be used again for
> copying data.

There is a pretty big difference between calling skb->destructor() and
freeing the buffer. My concern would be how do you deal with the
reference count issues that is going to bring up?

> There are no drivers setting the skb->destructor() in receive path
> upstream as of now but the plan is to get them there.

Okay. Got it.

Thanks.

- Alex

Re: [PATCH net-next] net: qualcomm: rmnet: Support recycling frames to real device

[David Miller]

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Fri, 27 Oct 2017 14:30:08 -0600

> For deaggregation, the real device receives a large linear skb and
> passes it on to rmnet. rmnet creates new skbs from this large frame.
> 
> If the real device supports recycling, it does not need to allocate
> the large skbs during packet reception and can instead reuse them.
> 
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>

I'm uncomfortable applying this until you also provide an example of
this skb->destructor mechanism actually being used in a driver.

Thank you.