Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-25 Thread Jonas Jensen
Thanks for giving feedback. On 21 August 2014 23:43, Michał Mirosław wrote: > This has implicit: if (!skb) BUG(); There should probably be a return > or continue inside the if (!skb). Fixed, see v6 update (broken out to separate patch) which now includes increment to RX head counter >

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-25 Thread Jonas Jensen
Thanks for giving feedback. On 21 August 2014 23:43, Michał Mirosław mir...@gmail.com wrote: This has implicit: if (!skb) BUG(); There should probably be a return or continue inside the if (!skb). Fixed, see v6 update (broken out to separate patch) which now includes increment to RX head

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-21 Thread Michał Mirosław
2014-08-19 16:49 GMT+02:00 Jonas Jensen : [...] > diff --git a/drivers/net/ethernet/moxa/moxart_ether.c > b/drivers/net/ethernet/moxa/moxart_ether.c > index aa45607..17c9f0e 100644 > --- a/drivers/net/ethernet/moxa/moxart_ether.c > +++ b/drivers/net/ethernet/moxa/moxart_ether.c > @@ -226,14

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-21 Thread Michał Mirosław
2014-08-19 16:49 GMT+02:00 Jonas Jensen jonas.jen...@gmail.com: [...] diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c index aa45607..17c9f0e 100644 --- a/drivers/net/ethernet/moxa/moxart_ether.c +++ b/drivers/net/ethernet/moxa/moxart_ether.c

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-20 Thread Jonas Jensen
On 19 August 2014 20:31, Eric Dumazet wrote: > Instead of : > > priv->rx_buf_size = RX_BUF_SIZE + > SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > > I think rx_buf_size can now be RX_BUF_SIZE You're right, that's now a remnant, see update in v5. > Another point is that

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-20 Thread Jonas Jensen
On 19 August 2014 20:31, Eric Dumazet eric.duma...@gmail.com wrote: Instead of : priv-rx_buf_size = RX_BUF_SIZE + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); I think rx_buf_size can now be RX_BUF_SIZE You're right, that's now a remnant, see update in v5. Another point

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-19 Thread Eric Dumazet
On Tue, 2014-08-19 at 16:49 +0200, Jonas Jensen wrote: > build_skb() is used to make skbs out of existing RX ring memory > which is bad because the RX ring is allocated only once, on probe. > Memory corruption occur because said memory is reclaimed, i.e. > __kfree_skb() (and eventually

[PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-19 Thread Jonas Jensen
build_skb() is used to make skbs out of existing RX ring memory which is bad because the RX ring is allocated only once, on probe. Memory corruption occur because said memory is reclaimed, i.e. __kfree_skb() (and eventually put_page()). Replace build_skb() with netdev_alloc_skb_ip_align() and use

[PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-19 Thread Jonas Jensen
build_skb() is used to make skbs out of existing RX ring memory which is bad because the RX ring is allocated only once, on probe. Memory corruption occur because said memory is reclaimed, i.e. __kfree_skb() (and eventually put_page()). Replace build_skb() with netdev_alloc_skb_ip_align() and use

Re: [PATCH v4 2/2] net: moxa: replace build_skb() with netdev_alloc_skb_ip_align() / memcpy()

2014-08-19 Thread Eric Dumazet
On Tue, 2014-08-19 at 16:49 +0200, Jonas Jensen wrote: build_skb() is used to make skbs out of existing RX ring memory which is bad because the RX ring is allocated only once, on probe. Memory corruption occur because said memory is reclaimed, i.e. __kfree_skb() (and eventually put_page()).