Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1266671 > unrolled thread

Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Started byEric Dumazet <eric.dumazet@gmail.com>
First post2015-11-10 19:00 +0100
Last post2015-11-11 15:10 +0100
Articles 11 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-10 19:00 +0100
    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-10 19:10 +0100
      Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-10 21:10 +0100
        Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-10 22:00 +0100
          Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-10 22:10 +0100
            Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-10 22:30 +0100
              Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-10 23:00 +0100
                Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Mason <slash.tmp@free.fr> - 2015-11-11 14:50 +0100
                  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 15:00 +0100
                    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 15:20 +0100
                  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 15:10 +0100

#1266671 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-11-10 19:00 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtln3-8aF-3@gated-at.bofh.it>
On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.

...

> +
> +static void nb8800_receive(struct net_device *dev, int i, int len)
> +{
> +	struct nb8800_priv *priv = netdev_priv(dev);
> +	struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
> +	struct page *page = priv->rx_bufs[i].page;
> +	int offset = priv->rx_bufs[i].offset;
> +	void *data = page_address(page) + offset;
> +	dma_addr_t dma = rxd->desc.s_addr;
> +	struct sk_buff *skb;
> +	int size;
> +	int err;
> +
> +	size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
> +
> +	skb = napi_alloc_skb(&priv->napi, size);
> +	if (!skb) {
> +		netdev_err(dev, "rx skb allocation failed\n");
> +		dev->stats.rx_dropped++;
> +		return;
> +	}
> +
> +	if (len <= RX_COPYBREAK) {
> +		dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
> +		memcpy(skb_put(skb, len), data, len);
> +		dma_sync_single_for_device(&dev->dev, dma, len,
> +					   DMA_FROM_DEVICE);
> +	} else {
> +		err = nb8800_alloc_rx(dev, i, true);
> +		if (err) {
> +			netdev_err(dev, "rx buffer allocation failed\n");
> +			dev->stats.rx_dropped++;
> +			return;
> +		}
> +
> +		dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
> +		memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
> +		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
> +				offset + RX_COPYHDR, len - RX_COPYHDR,
> +				RX_BUF_SIZE);
> +	}
> +
> +	skb->protocol = eth_type_trans(skb, dev);
> +	netif_receive_skb(skb);
> +}
> +


Any reason you do not use napi_gro_receive(&priv->napi, skb) instead of
netif_receive_skb() ?

I see you correctly use late napi_complete_done(napi, work), but this
matters only if napi_gro_receive() was used.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1266683 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-10 19:10 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtlwJ-8tw-1@gated-at.bofh.it>
In reply to#1266671
Eric Dumazet <eric.dumazet@gmail.com> writes:

> On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>> It is an almost complete rewrite of a driver originally found in
>> a Sigma Designs 2.6.22 tree.
>
> ...
>
>> +
>> +static void nb8800_receive(struct net_device *dev, int i, int len)
>> +{
>> +	struct nb8800_priv *priv = netdev_priv(dev);
>> +	struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
>> +	struct page *page = priv->rx_bufs[i].page;
>> +	int offset = priv->rx_bufs[i].offset;
>> +	void *data = page_address(page) + offset;
>> +	dma_addr_t dma = rxd->desc.s_addr;
>> +	struct sk_buff *skb;
>> +	int size;
>> +	int err;
>> +
>> +	size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
>> +
>> +	skb = napi_alloc_skb(&priv->napi, size);
>> +	if (!skb) {
>> +		netdev_err(dev, "rx skb allocation failed\n");
>> +		dev->stats.rx_dropped++;
>> +		return;
>> +	}
>> +
>> +	if (len <= RX_COPYBREAK) {
>> +		dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
>> +		memcpy(skb_put(skb, len), data, len);
>> +		dma_sync_single_for_device(&dev->dev, dma, len,
>> +					   DMA_FROM_DEVICE);
>> +	} else {
>> +		err = nb8800_alloc_rx(dev, i, true);
>> +		if (err) {
>> +			netdev_err(dev, "rx buffer allocation failed\n");
>> +			dev->stats.rx_dropped++;
>> +			return;
>> +		}
>> +
>> +		dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
>> +		memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
>> +		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
>> +				offset + RX_COPYHDR, len - RX_COPYHDR,
>> +				RX_BUF_SIZE);
>> +	}
>> +
>> +	skb->protocol = eth_type_trans(skb, dev);
>> +	netif_receive_skb(skb);
>> +}
>> +
>
> Any reason you do not use napi_gro_receive(&priv->napi, skb) instead of
> netif_receive_skb() ?

Because I haven't been following the netdev list closely for the last
five years, and no documentation I read mentioned this function.  I can
certainly change it.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266755

FromDavid Miller <davem@davemloft.net>
Date2015-11-10 21:10 +0100
Message-ID<qtnoS-1h5-13@gated-at.bofh.it>
In reply to#1266683
From: Måns Rullgård <mans@mansr.com>
Date: Tue, 10 Nov 2015 18:05:15 +0000

> Because I haven't been following the netdev list closely for the last
> five years, and no documentation I read mentioned this function.  I can
> certainly change it.

It is always advisable to mimick what other drivers do and use them as
a reference, rather than depend upon documentation which by definition
is always going to be out of sync with the source tree.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266783 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-10 22:00 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtobg-1Ah-5@gated-at.bofh.it>
In reply to#1266755
David Miller <davem@davemloft.net> writes:

> From: Måns Rullgård <mans@mansr.com>
> Date: Tue, 10 Nov 2015 18:05:15 +0000
>
>> Because I haven't been following the netdev list closely for the last
>> five years, and no documentation I read mentioned this function.  I can
>> certainly change it.
>
> It is always advisable to mimick what other drivers do and use them as
> a reference, rather than depend upon documentation which by definition
> is always going to be out of sync with the source tree.

Sure.  The trick is to pick the right driver(s) to use as reference.
Quite a few of them don't use that function.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266787

FromDavid Miller <davem@davemloft.net>
Date2015-11-10 22:10 +0100
Message-ID<qtokW-1T4-19@gated-at.bofh.it>
In reply to#1266783
From: Måns Rullgård <mans@mansr.com>
Date: Tue, 10 Nov 2015 20:53:19 +0000

> David Miller <davem@davemloft.net> writes:
> 
>> From: Måns Rullgård <mans@mansr.com>
>> Date: Tue, 10 Nov 2015 18:05:15 +0000
>>
>>> Because I haven't been following the netdev list closely for the last
>>> five years, and no documentation I read mentioned this function.  I can
>>> certainly change it.
>>
>> It is always advisable to mimick what other drivers do and use them as
>> a reference, rather than depend upon documentation which by definition
>> is always going to be out of sync with the source tree.
> 
> Sure.  The trick is to pick the right driver(s) to use as reference.
> Quite a few of them don't use that function.

If you really are stumped on this matter, start at least with the
ixgbe driver.  In fact pretty much every Intel ethernet driver is
a reasonable reference.  Others to check out are bnx2x and mlx5.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266791 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-10 22:30 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtoEi-20c-15@gated-at.bofh.it>
In reply to#1266787
David Miller <davem@davemloft.net> writes:

> From: Måns Rullgård <mans@mansr.com>
> Date: Tue, 10 Nov 2015 20:53:19 +0000
>
>> David Miller <davem@davemloft.net> writes:
>> 
>>> From: Måns Rullgård <mans@mansr.com>
>>> Date: Tue, 10 Nov 2015 18:05:15 +0000
>>>
>>>> Because I haven't been following the netdev list closely for the last
>>>> five years, and no documentation I read mentioned this function.  I can
>>>> certainly change it.
>>>
>>> It is always advisable to mimick what other drivers do and use them as
>>> a reference, rather than depend upon documentation which by definition
>>> is always going to be out of sync with the source tree.
>> 
>> Sure.  The trick is to pick the right driver(s) to use as reference.
>> Quite a few of them don't use that function.
>
> If you really are stumped on this matter, start at least with the
> ixgbe driver.  In fact pretty much every Intel ethernet driver is
> a reasonable reference.  Others to check out are bnx2x and mlx5.

Even ixgbe uses napi_complete() while netdevice.h says one should
"consider using napi_complete_done() instead."  Did the author consider
it and decide not to, or has the driver simply not been updated?

As for the napi_gro_receive() function, calling that instead of
netif_receive_skb() is easy enough, or are there other things I should
be doing in addition?

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266807

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-11-10 23:00 +0100
Message-ID<qtp7k-2b1-11@gated-at.bofh.it>
In reply to#1266791
On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:

> Even ixgbe uses napi_complete() while netdevice.h says one should
> "consider using napi_complete_done() instead."  Did the author consider
> it and decide not to, or has the driver simply not been updated?

napi_complete_done() is quite new, very few drivers use it.

It still requires a tuning (/sys/class/net/ethX/gro_flush_timeout)

> 
> As for the napi_gro_receive() function, calling that instead of
> netif_receive_skb() is easy enough, or are there other things I should
> be doing in addition?

Nothing comes to mind, if you already have a NAPI context,
napi_gro_receive() is the way to go...



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267198

FromMason <slash.tmp@free.fr>
Date2015-11-11 14:50 +0100
Message-ID<qtDWH-3rN-23@gated-at.bofh.it>
In reply to#1266807
On 10/11/2015 22:51, Eric Dumazet wrote:

> On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
> 
>> Even ixgbe uses napi_complete() while netdevice.h says one should
>> "consider using napi_complete_done() instead."  Did the author consider
>> it and decide not to, or has the driver simply not been updated?
> 
> napi_complete_done() is quite new, very few drivers use it.

I was hoping to back-port this driver for my platform. Are the recent
features used in the driver available in 4.1? What about 3.14?

Regards.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267203 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-11 15:00 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtE6n-3vE-21@gated-at.bofh.it>
In reply to#1267198
Mason <slash.tmp@free.fr> writes:

> On 10/11/2015 22:51, Eric Dumazet wrote:
>
>> On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
>> 
>>> Even ixgbe uses napi_complete() while netdevice.h says one should
>>> "consider using napi_complete_done() instead."  Did the author consider
>>> it and decide not to, or has the driver simply not been updated?
>> 
>> napi_complete_done() is quite new, very few drivers use it.
>
> I was hoping to back-port this driver for my platform. Are the recent
> features used in the driver available in 4.1? What about 3.14?

xmit_more was added in 3.18, napi_complete_done() in 3.19.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267213

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-11-11 15:20 +0100
Message-ID<qtEpH-3Sx-13@gated-at.bofh.it>
In reply to#1267203
On Wed, 2015-11-11 at 13:54 +0000, Måns Rullgård wrote:
> Mason <slash.tmp@free.fr> writes:
> 
> > On 10/11/2015 22:51, Eric Dumazet wrote:
> >
> >> On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
> >> 
> >>> Even ixgbe uses napi_complete() while netdevice.h says one should
> >>> "consider using napi_complete_done() instead."  Did the author consider
> >>> it and decide not to, or has the driver simply not been updated?
> >> 
> >> napi_complete_done() is quite new, very few drivers use it.
> >
> > I was hoping to back-port this driver for my platform. Are the recent
> > features used in the driver available in 4.1? What about 3.14?
> 
> xmit_more was added in 3.18, napi_complete_done() in 3.19.

xmit_more is a hint, you can simply consider skb->xmit_more being
replaced by 0 on old kernels if you want to backport.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267206

FromEric Dumazet <eric.dumazet@gmail.com>
Date2015-11-11 15:10 +0100
Message-ID<qtEg2-3OD-9@gated-at.bofh.it>
In reply to#1267198
On Wed, 2015-11-11 at 14:41 +0100, Mason wrote:
> On 10/11/2015 22:51, Eric Dumazet wrote:
> 
> > On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
> > 
> >> Even ixgbe uses napi_complete() while netdevice.h says one should
> >> "consider using napi_complete_done() instead."  Did the author consider
> >> it and decide not to, or has the driver simply not been updated?
> > 
> > napi_complete_done() is quite new, very few drivers use it.
> 
> I was hoping to back-port this driver for my platform. Are the recent
> features used in the driver available in 4.1? What about 3.14?

Sure, this is all available 

napi_complete_done() can be backported on old kernels as a
napi_complete() 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web