* Re: [PATCH RFC 3/5] net:stmmac: ensure we reclaim all dirty descriptors.
From: Eric Dumazet @ 2013-10-21 18:49 UTC (permalink / raw)
To: Jimmy PERCHET; +Cc: Giuseppe CAVALLARO, netdev
In-Reply-To: <1382380327.3284.77.camel@edumazet-glaptop.roam.corp.google.com>
On Mon, 2013-10-21 at 11:32 -0700, Eric Dumazet wrote:
> On Mon, 2013-10-21 at 15:10 +0200, Jimmy PERCHET wrote:
> > Hello Peppe,
> >
>
> > I can reproduce this problem by issuing 9KiB jumbo frames on 10MBit/s link.
> > If socket's wmemory size is about 500kiB (or less), the transfer stall.
> > (I guess it is reproducible with 1500o frames by decreasing
> > socket's wmemory to 90KB)
> > Re-arming the timer fix this behaviour.
> >
> > Here my understanding of this issue :
> > With 9KiB frames and 500kiB of wmemory, only 60 frames can be
> > prepared in a row. It is below the tx coalescence threshold,
> > so there will be no interrupt. When the tx coalescence timer
> > expires (40ms after), only five descriptors have to be
> > freed (9000*5 @ 10Mbit/s = 34ms), it is not enough to reach
> > the socket's wake-up threshold. We get into a deadlock :
> > *Socket is waiting for free buffers before performing new transfer.
> > *Driver is waiting for new transfer before performing cleanup.
> >
> > Maybe, it is not a real life use-case, and is not worth
> > a patch. What do you think ?
> >
>
> I think there is probably a bug in the driver, a race of some sort,
> and it would be better to find it and fix it ;)
>
coalesce params should not be hardcoded, but depend on link speed and
mtu.
On 10Mbits, and MTU=9000 there is really no point using coalescing !
^ permalink raw reply
* Re: [PATCH RFC 3/5] net:stmmac: ensure we reclaim all dirty descriptors.
From: Eric Dumazet @ 2013-10-21 18:32 UTC (permalink / raw)
To: Jimmy PERCHET; +Cc: Giuseppe CAVALLARO, netdev
In-Reply-To: <526527DE.5060906@parrot.com>
On Mon, 2013-10-21 at 15:10 +0200, Jimmy PERCHET wrote:
> Hello Peppe,
>
> I can reproduce this problem by issuing 9KiB jumbo frames on 10MBit/s link.
> If socket's wmemory size is about 500kiB (or less), the transfer stall.
> (I guess it is reproducible with 1500o frames by decreasing
> socket's wmemory to 90KB)
> Re-arming the timer fix this behaviour.
>
> Here my understanding of this issue :
> With 9KiB frames and 500kiB of wmemory, only 60 frames can be
> prepared in a row. It is below the tx coalescence threshold,
> so there will be no interrupt. When the tx coalescence timer
> expires (40ms after), only five descriptors have to be
> freed (9000*5 @ 10Mbit/s = 34ms), it is not enough to reach
> the socket's wake-up threshold. We get into a deadlock :
> *Socket is waiting for free buffers before performing new transfer.
> *Driver is waiting for new transfer before performing cleanup.
>
> Maybe, it is not a real life use-case, and is not worth
> a patch. What do you think ?
>
I think there is probably a bug in the driver, a race of some sort,
and it would be better to find it and fix it ;)
^ permalink raw reply
* Re: [PATCH RFC 5/5] net:stmmac: asynchronous tx_clean
From: Eric Dumazet @ 2013-10-21 18:24 UTC (permalink / raw)
To: Jimmy PERCHET; +Cc: Giuseppe CAVALLARO, netdev
In-Reply-To: <52656CE1.1060703@parrot.com>
On Mon, 2013-10-21 at 20:05 +0200, Jimmy PERCHET wrote:
> I understand your point. Nevertheless I think it is still possible
> to avoid serialization, and therefore increase performance, even if
> completions must remain in softirq. What do you think ?
I think this will break over time. This kind of 'optimizations' work on
a particular workload, and thats it. It reminds me the 'skb recycle'
thing.
The workload you try to optimize might conflict with other workloads.
Have you tried to reduce the 64 value in netif_napi_add() ?
>
> In my patch I tried to avoid any race condition. (by updating both
> descriptor's cursors only once, for instance)
> Could you explain the possible race you see ?
Well, take a look at netif_queue_stopped(), netif_wake_queue(),
netif_queue_stopped() calls for a start.
Its really hard to make start_xmit() lockless (versus TX completion).
Adding atomic_t wont be enough (and btw thats often not needed at all)
drivers/net/ethernet/broadcom/tg3.c is a good reference if you really
want to do that properly.
^ permalink raw reply
* Re: [PATCH RFC 5/5] net:stmmac: asynchronous tx_clean
From: Jimmy PERCHET @ 2013-10-21 18:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Giuseppe CAVALLARO, netdev, Jimmy Perchet
In-Reply-To: <1382373005.3284.61.camel@edumazet-glaptop.roam.corp.google.com>
On 21/10/2013 18:30, Eric Dumazet wrote:
> On Mon, 2013-10-21 at 15:52 +0200, Giuseppe CAVALLARO wrote:
>> Hello Jimmy
>>
>> On 10/16/2013 5:24 PM, Jimmy Perchet wrote:
>>> Tx descriptor's cleanup and preparation are serialized, which is not necessary
>>> and decrease performance.
>>> In addition TX descriptor's cleanup is performed on NET_RX softirq, this is
>>> confusing.
>>
>> hmm, here you are changing the logic behind the tx/rx processes.
>>
>> As done in many drivers, the stmmac cleans the tx resources in
>> NAPI context and this is not a confuse approach ;-).
>>
>> It gave me some performance improvements especially on TCP benchmarks.
>>
>>> This patch unserialize tx descriptor's cleanup and preparation
>>> and defer cleanup in workqueue.
>>
>> So you decide to use workqueue and I kindly ask you to give me more
>> details about the performance improvements (UDP/TCP) and cpu usage.
>>
>> I can try to do some tests on my side too. This could take a while
>> unfortunately.
>
> Anyway this patch is buggy.
>
> 1) Removing tx_lock spinlock in TX completion adds a race in
> stmmac_xmit()
>
> 2) Generally speaking, we should not rely on a work queue to perform TX
> completions.
>
> Think about being flooded by incoming frames.
>
> Work queue could never be scheduled.
>
>
I understand your point. Nevertheless I think it is still possible
to avoid serialization, and therefore increase performance, even if
completions must remain in softirq. What do you think ?
In my patch I tried to avoid any race condition. (by updating both
descriptor's cursors only once, for instance)
Could you explain the possible race you see ?
Best Regards,
Jimmy
^ permalink raw reply
* Re: [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Mariusz Ceier @ 2013-10-21 17:59 UTC (permalink / raw)
To: Mugunthan V N
Cc: David S. Miller, Lad Prabhakar, Jingoo Han, Jiri Pirko, netdev,
linux-kernel
In-Reply-To: <52656971.3020509@ti.com>
Yes of course ;) This was my first patch for the kernel. I will
remember about v2 tag next time.
Thanks,
Mariusz Ceier
On 21 October 2013 19:50, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Monday 21 October 2013 11:15 PM, Mariusz Ceier wrote:
>> When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't,
>> emac_dev_mcast_set should only enable RX of multicasts and reset
>> MACHASH registers.
>>
>> It does this, but afterwards it either sets up multicast MACs
>> filtering or disables RX of multicasts and resets MACHASH registers
>> again, rendering IFF_ALLMULTI flag useless.
>>
>> This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and
>> disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set.
>>
>> Tested with kernel 2.6.37.
>>
>> Signed-off-by: Mariusz Ceier <mceier+kernel@gmail.com>
>> ---
> Can you add [PATCH v2] in your subject in future so that it will be
> easier to the maintainer to pick the latest version of the patch.
>
> Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
>
> Regards
> Mugunthan V N
^ permalink raw reply
* Re: [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Mugunthan V N @ 2013-10-21 17:50 UTC (permalink / raw)
To: Mariusz Ceier
Cc: David S. Miller, Lad Prabhakar, Jingoo Han, Jiri Pirko, netdev,
linux-kernel
In-Reply-To: <1382377504-24688-1-git-send-email-mceier+kernel@gmail.com>
On Monday 21 October 2013 11:15 PM, Mariusz Ceier wrote:
> When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't,
> emac_dev_mcast_set should only enable RX of multicasts and reset
> MACHASH registers.
>
> It does this, but afterwards it either sets up multicast MACs
> filtering or disables RX of multicasts and resets MACHASH registers
> again, rendering IFF_ALLMULTI flag useless.
>
> This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and
> disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set.
>
> Tested with kernel 2.6.37.
>
> Signed-off-by: Mariusz Ceier <mceier+kernel@gmail.com>
> ---
Can you add [PATCH v2] in your subject in future so that it will be
easier to the maintainer to pick the latest version of the patch.
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
Regards
Mugunthan V N
^ permalink raw reply
* [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Mariusz Ceier @ 2013-10-21 17:45 UTC (permalink / raw)
To: David S. Miller, Mugunthan V N, Lad Prabhakar, Jingoo Han,
Jiri Pirko
Cc: netdev, linux-kernel, Mariusz Ceier
In-Reply-To: <5265640C.4070201@cogentembedded.com>
When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't,
emac_dev_mcast_set should only enable RX of multicasts and reset
MACHASH registers.
It does this, but afterwards it either sets up multicast MACs
filtering or disables RX of multicasts and resets MACHASH registers
again, rendering IFF_ALLMULTI flag useless.
This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and
disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set.
Tested with kernel 2.6.37.
Signed-off-by: Mariusz Ceier <mceier+kernel@gmail.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 67df09e..6a32ef9 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -876,8 +876,7 @@ static void emac_dev_mcast_set(struct net_device *ndev)
netdev_mc_count(ndev) > EMAC_DEF_MAX_MULTICAST_ADDRESSES) {
mbp_enable = (mbp_enable | EMAC_MBP_RXMCAST);
emac_add_mcast(priv, EMAC_ALL_MULTI_SET, NULL);
- }
- if (!netdev_mc_empty(ndev)) {
+ } else if (!netdev_mc_empty(ndev)) {
struct netdev_hw_addr *ha;
mbp_enable = (mbp_enable | EMAC_MBP_RXMCAST);
--
1.8.4
^ permalink raw reply related
* Re: [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Sergei Shtylyov @ 2013-10-21 17:27 UTC (permalink / raw)
To: Mariusz Ceier, David S. Miller, Mugunthan V N, Lad Prabhakar,
Jingoo Han, Jiri Pirko
Cc: netdev, linux-kernel
In-Reply-To: <1382374777-24200-1-git-send-email-mceier+kernel@gmail.com>
Hello.
On 10/21/2013 08:59 PM, Mariusz Ceier wrote:
> When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't,
> emac_dev_mcast_set should only enable RX of multicasts and reset
> MACHASH registers.
> It does this, but afterwards it either sets up multicast MACs
> filtering or disables RX of multicasts and resets MACHASH registers
> again, rendering IFF_ALLMULTI flag useless.
> This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and
> disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set.
> Tested with kernel 2.6.37.
> Signed-off-by: Mariusz Ceier <mceier+kernel@gmail.com>
> ---
> drivers/net/ethernet/ti/davinci_emac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index 67df09e..ff3bf0e 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -876,7 +876,7 @@ static void emac_dev_mcast_set(struct net_device *ndev)
> netdev_mc_count(ndev) > EMAC_DEF_MAX_MULTICAST_ADDRESSES) {
> mbp_enable = (mbp_enable | EMAC_MBP_RXMCAST);
> emac_add_mcast(priv, EMAC_ALL_MULTI_SET, NULL);
> - }
> + } else
> if (!netdev_mc_empty(ndev)) {
It should be:
} else if (!netdev_mc_empty(ndev)) {
WBR, Sergei
^ permalink raw reply
* [PATCH] mac802154: correct a typo in ieee802154_alloc_device() prototype
From: Alexandre Belloni @ 2013-10-21 17:09 UTC (permalink / raw)
To: David S. Miller
Cc: alex.bluesman.smirnov, netdev, linux-kernel, Alexandre Belloni
This has no other impact than a cosmetic one.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
include/net/mac802154.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index d0d11df..807d6b7 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -133,7 +133,7 @@ struct ieee802154_ops {
/* Basic interface to register ieee802154 device */
struct ieee802154_dev *
-ieee802154_alloc_device(size_t priv_data_lex, struct ieee802154_ops *ops);
+ieee802154_alloc_device(size_t priv_data_len, struct ieee802154_ops *ops);
void ieee802154_free_device(struct ieee802154_dev *dev);
int ieee802154_register_device(struct ieee802154_dev *dev);
void ieee802154_unregister_device(struct ieee802154_dev *dev);
--
1.8.3.2
^ permalink raw reply related
* [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Mariusz Ceier @ 2013-10-21 16:59 UTC (permalink / raw)
To: David S. Miller, Mugunthan V N, Lad Prabhakar, Jingoo Han,
Jiri Pirko
Cc: netdev, linux-kernel, Mariusz Ceier
When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't,
emac_dev_mcast_set should only enable RX of multicasts and reset
MACHASH registers.
It does this, but afterwards it either sets up multicast MACs
filtering or disables RX of multicasts and resets MACHASH registers
again, rendering IFF_ALLMULTI flag useless.
This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and
disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set.
Tested with kernel 2.6.37.
Signed-off-by: Mariusz Ceier <mceier+kernel@gmail.com>
---
drivers/net/ethernet/ti/davinci_emac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 67df09e..ff3bf0e 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -876,7 +876,7 @@ static void emac_dev_mcast_set(struct net_device *ndev)
netdev_mc_count(ndev) > EMAC_DEF_MAX_MULTICAST_ADDRESSES) {
mbp_enable = (mbp_enable | EMAC_MBP_RXMCAST);
emac_add_mcast(priv, EMAC_ALL_MULTI_SET, NULL);
- }
+ } else
if (!netdev_mc_empty(ndev)) {
struct netdev_hw_addr *ha;
--
1.8.4
^ permalink raw reply related
* Re: [PATCH RFC 5/5] net:stmmac: asynchronous tx_clean
From: Eric Dumazet @ 2013-10-21 16:30 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: Jimmy Perchet, netdev
In-Reply-To: <5265318C.5050307@st.com>
On Mon, 2013-10-21 at 15:52 +0200, Giuseppe CAVALLARO wrote:
> Hello Jimmy
>
> On 10/16/2013 5:24 PM, Jimmy Perchet wrote:
> > Tx descriptor's cleanup and preparation are serialized, which is not necessary
> > and decrease performance.
> > In addition TX descriptor's cleanup is performed on NET_RX softirq, this is
> > confusing.
>
> hmm, here you are changing the logic behind the tx/rx processes.
>
> As done in many drivers, the stmmac cleans the tx resources in
> NAPI context and this is not a confuse approach ;-).
>
> It gave me some performance improvements especially on TCP benchmarks.
>
> > This patch unserialize tx descriptor's cleanup and preparation
> > and defer cleanup in workqueue.
>
> So you decide to use workqueue and I kindly ask you to give me more
> details about the performance improvements (UDP/TCP) and cpu usage.
>
> I can try to do some tests on my side too. This could take a while
> unfortunately.
Anyway this patch is buggy.
1) Removing tx_lock spinlock in TX completion adds a race in
stmmac_xmit()
2) Generally speaking, we should not rely on a work queue to perform TX
completions.
Think about being flooded by incoming frames.
Work queue could never be scheduled.
^ permalink raw reply
* Re: [PATCH RFC 4/5] net:stmmac: fix jumbo frame handling.
From: Jimmy PERCHET @ 2013-10-21 16:28 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, jimmy.perchet
In-Reply-To: <52652EE7.2060500@st.com>
On 21/10/2013 15:40, Giuseppe CAVALLARO wrote:
> On 10/16/2013 5:24 PM, Jimmy Perchet wrote:
>> This patch addresses several issues which prevent jumbo frames from working properly :
>> .jumbo frames' last descriptor was not closed
>> .several confusion regarding descriptor's max buffer size
>> .frags could not be jumbo
>>
>> Signed-off-by: Jimmy Perchet <jimmy.perchet@parrot.com>
>
>
> Jimmy, thx for thi patch. BElow some my first notes.
Thanks a lot for this first review.
> I'll continue to look at the patch to verify if I missed
> soemthing. I kindly ask you, for the next version, to add
> more comments especially in the function to prepare the
> tx desc in order to help me on reviewing.
Sure ;)
I hope do v2 by next week.
I'm OK with most of your comments. Some additional
notes below:
>> }
>> @@ -81,7 +81,7 @@ static inline void ndesc_end_tx_desc_on_ring(struct dma_desc *p, int ter)
>>
>> static inline void norm_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
>> {
>> - if (unlikely(len > BUF_SIZE_2KiB)) {
>> + if (unlikely(len >= BUF_SIZE_2KiB)) {
>
> we cannot manage a size of 2048 on normal desc
>
> Pls you should verify to not break the back-compatibility.
IMHO, this actually fix the problem you think I create.
In current code, if len is equal to 2048, buffer1_size is set to 2048,
this is wrong because the max size is actually 2047...
>
>> p->des01.etx.buffer1_size = BUF_SIZE_2KiB - 1;
>> p->des01.etx.buffer2_size = len - p->des01.etx.buffer1_size;
>> } else
>>
>> static void stmmac_refill_desc3(void *priv_ptr, struct dma_desc *p)
>> {
>> @@ -103,13 +90,13 @@ static void stmmac_refill_desc3(void *priv_ptr, struct dma_desc *p)
>> if (unlikely(priv->plat->has_gmac))
>> /* Fill DES3 in case of RING mode */
>> if (priv->dma_buf_sz >= BUF_SIZE_8KiB)
>> - p->des3 = p->des2 + BUF_SIZE_8KiB;
>> + p->des3 = p->des2 + BUF_SIZE_8KiB - 1;
>
> is it correct? can you check?
The actual buffer's max size is 8191, so, in ring mode,
the second buffer must start at p->des2 + 8191.
>> - priv->cur_tx++;
>> + priv->cur_tx += nb_desc;
>
> can we avoid to use the nb_desc?
Actually, it is a preparation for my 5th patch : I want to write cur_tx only once.
I can split this.
Best Regards,
Jimmy
^ permalink raw reply
* Stale IPv6 address accumulation on linux 3.2.17
From: Templin, Fred L @ 2013-10-21 15:50 UTC (permalink / raw)
To: netdev@vger.kernel.org
In-Reply-To: <loom.20131021T165114-650@post.gmane.org>
Hi,
On linux 3.2.17, I have a host that configures IPv6 addresses on
an eth0 interface based on Router Advertisements received from an
on-link linux box configured as an IPv6 router and running radvd.
When the host gets an RA, it configures both an EUI-64-based IPv6
address and an IPv6 privacy address, so it has two IPv6 addresses.
But, if I leave the host up for long periods of time, it seems to
accumulate additional IPv6 addresses - perhaps these are stale
IPv6 privacy addresses?
Is this known behavior, and if so is there a way to turn it off?
Or, perhaps this was a known bug that has been corrected in more
recent linux kernel versions?
Thanks - Fred
fred.l.templin@boeing.com
^ permalink raw reply
* Re: [BUG] 3.12.0-rcX IPv6 panic
From: Hannes Frederic Sowa @ 2013-10-21 15:52 UTC (permalink / raw)
To: Bob Tracy; +Cc: linux-kernel, netdev
In-Reply-To: <20131021131846.GA5769@gherkin.frus.com>
Hi!
On Mon, Oct 21, 2013 at 08:18:46AM -0500, Bob Tracy wrote:
> Actually, a regression: the 3.11 kernel is rock-solid stable on my
> Alpha.
>
> Beginning with 3.12.0-rc1, I can reliably trigger a kernel panic by
> executing the gogo6.net "gw6c" IPv6 client program. If the networking
> layer is active, an "Oops" will eventually (within a day) occur regardless
> of whether I attempt to run "gw6c". 3.12.0-rcX is stable as long as I
> leave networking completely disabled. The error has persisted up through
> -rc6. Apologies for not mentioning this earlier, but the state of my
> PWS-433au has been questionable, and I wanted to make sure I had a
> legitimate bug sighting.
>
> I'll have to transcribe the panic backtrace by hand: nothing makes it
> into any of the system logs :-(. I *can* recall that every backtrace
> I've seen thus far has included one of the skb_copy() variants near the
> top of the list (first or second function).
Try to capture the panic via serial console. Otherwise a picture
would give us a first hint. Please watch out for lines like
skb_(over|under)_panic.
gw6c is a tunnel client? Can you post ip -6 tunnel ls?
(Also please send networking bugs to netdev@).
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Daniel Borkmann @ 2013-10-21 15:48 UTC (permalink / raw)
To: Daniel Wagner
Cc: Eric W. Biederman, pablo-Cap9r6Oaw4JrovVCs/uTlw,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <526543A2.2040901-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
On 10/21/2013 05:09 PM, Daniel Wagner wrote:
> On 10/19/2013 08:16 AM, Daniel Borkmann wrote:
>> On 10/19/2013 01:21 AM, Eric W. Biederman wrote:
>>
>>> I am coming to this late. But two concrete suggestions.
>>>
>>> 1) process groups and sessions don't change as frequently as pids.
>>>
>>> 2) It is possible to put a set of processes in their own network
>>> namespace and pipe just the packets you want those processes to
>>> use into that network namespace. Using an ingress queueing filter
>>> makes that process very efficient even if you have to filter by port.
>>
>> Actually in our case we're filtering outgoing traffic, based on which
>> local socket that originated from; so you wouldn't need all of that
>> construct. Also, you wouldn't even need to have an a-prio knowledge of
>> the application internals regarding their use of particular use of ports
>> or protocols. I don't think that such a setup will have the same
>> efficiency, ease of use, and power to distinguish the application the
>> traffic came from in such a lightweight, protocol independent and easy way.
>
> Sorry for beeing late as well (and also stupid question)
>
> Couldn't you use something from the LSM? I mean you allow the
> application to create the socket etc and then block later
> the traffic originated from that socket. Wouldn't it make
> more sense to block early?
I gave one simple example for blocking in the commit message,
that's true, but it is not limited to that, meaning we can have
much different scenarios/policies that netfilter allows us than
just blocking, e.g. fine grained settings where applications are
allowed to connect/send traffic to, application traffic marking/
conntracking, application-specific packet mangling, and so on,
just think of the whole netfilter universe.
^ permalink raw reply
* DEVELLOP YOUR INTEREST PLEASE
From: MRS MUSA SAMORA @ 2013-10-21 15:18 UTC (permalink / raw)
FROM MRS MUSA SAMORA
Hello
My name is Mrs Samora Musa,I have been suffering from Ovarian cancer disease and the doctor says that i have just few days to leave.I am from Libya, but based in Burkina Faso,Africa since ten years ago as a business woman dealing with cocoa exportation,now that i am about to end the race like this,without any family members and no child.I have $1.1 Million US DOLLARS in Bank of Africa(BOA)Burkina Faso which i instructed the bank to give African union leaders to help sick people around Africa.But my mind is not at rest because i am writing this letter now through the help of my nurse beside me here in my hospital room.
I also have $2.1 Million US Dollars in another bank in Burkina Faso which i want you to claim from the bank and use help less privileged people in your country,but you must assure me that you will take only 40% of the total money and give the rest 60% to the orphanage home in your country for my heart to rest.
Upon the receipt of your email that you are willing and capable to execute my plan, i will instruct the bank management to make the immediate transfer into your account.
Sincerely,
Mrs Samora Musa.
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Daniel Wagner @ 2013-10-21 15:09 UTC (permalink / raw)
To: Daniel Borkmann, Eric W. Biederman
Cc: pablo, netfilter-devel, netdev, Tejun Heo, cgroups
In-Reply-To: <526231E0.6060903@redhat.com>
Hi Daniel
On 10/19/2013 08:16 AM, Daniel Borkmann wrote:
> On 10/19/2013 01:21 AM, Eric W. Biederman wrote:
>
>> I am coming to this late. But two concrete suggestions.
>>
>> 1) process groups and sessions don't change as frequently as pids.
>>
>> 2) It is possible to put a set of processes in their own network
>> namespace and pipe just the packets you want those processes to
>> use into that network namespace. Using an ingress queueing filter
>> makes that process very efficient even if you have to filter by port.
>
> Actually in our case we're filtering outgoing traffic, based on which
> local socket that originated from; so you wouldn't need all of that
> construct. Also, you wouldn't even need to have an a-prio knowledge of
> the application internals regarding their use of particular use of ports
> or protocols. I don't think that such a setup will have the same
> efficiency, ease of use, and power to distinguish the application the
> traffic came from in such a lightweight, protocol independent and easy way.
Sorry for beeing late as well (and also stupid question)
Couldn't you use something from the LSM? I mean you allow the
application to create the socket etc and then block later
the traffic originated from that socket. Wouldn't it make
more sense to block early?
cheers,
daniel
^ permalink raw reply
* Re: unmanaged L2TPv3 ethernet pseudowire Cisco <=> Linux
From: Pierre Desvaux @ 2013-10-21 15:08 UTC (permalink / raw)
To: netdev
In-Reply-To: <51535D16.4080207@katalix.com>
James Chapman <jchapman <at> katalix.com> writes:
>
> On 27/03/13 20:08, Tomas Agartz wrote:
> > On Tue, 26 Mar 2013, James Chapman wrote:
> >
> >> The issue is that Linux and Cisco use a different default for the
> >> L2SpecificSublayer header setting and neither implementation provides
> >> a config option to change its setting. The Linux default is to use
> >> the Default L2SpecificSublayer as defined in the RFC. Unfortunately
> >> the Cisco default is to use no L2SpecificSublayer.
> >>
> >> The kernel already has an API to allow the L2SpecificSublayer setting
> >> to be configured. The missing piece is an iproute2 l2tp config option
> >> to configure it. I'll work on an iproute2 patch now to allow this
> >> setting to be configured.
> >
> > I patched my iproute2 with your patch and now my tunnel is working.
> > Thank you! :)
>
> Great. Thanks for reporting back.
>
> >> For unmanaged tunnels, these parameters must be manually configured
> >> consistently at each side. Both Cisco and Linux default to use no
> >> cookies and both already have config parameters to set cookie
> >> parameters, if needed. However, for L2SpecificSublayer this isn't the
> >> case. We need to add a config option on the Linux side to force the
> >> same setting as Cisco is using.
> >
> > Does the API in the kernel allow you to set the cookie? In that case it
> > seems like a good idea to add that to iproute2 as well?
>
> It is already supported. See the cookie and peer_cookie parameters of ip
> l2tp add session.
>
> ip l2tp help
> or
> man ip-l2tp
>
> James
>
>
Hi,
I have tried an other solution to bypass this issue.
I put a 4 bytes cookie in the paquets sent by the Cisco. It looks like this:
[IPv4][L2TPv3][Cookie][payload]
With value 0, the cookie is seen by the Linux as a L2SpecificSublayer with
Sbit at 0. Wich means ignore the value of the sequence number in
L2SpecificSublayer so Linux accepts it. Linux replies automaticaly with Sbit
0 to Cisco.
Cisco is as well configured to accept a 4 bytes cookie, the
L2SpecificSublayer is now accepted as a cookie.
To configure Cisco:
xconnect 192.168.0.1 200 encapsulation l2tpv3 manual pw-class tlund
l2tp id 200 200
l2tp cookie local 4 0
l2tp cookie remote 4 0
Pierre
^ permalink raw reply
* Re: [PATCH net] bridge: clean the nf_bridge status when forwarding the skb
From: Antonio Quartulli @ 2013-10-21 15:06 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Pablo Neira Ayuso, Antonio Quartulli, David S. Miller,
netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <526541A0.4000504@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]
On Mon, Oct 21, 2013 at 11:00:48AM -0400, Vlad Yasevich wrote:
> >>
> >> Looking at other encapsulators (PPP, iptunnel, VXLAN), they do
> >> nf_reset() on input. Would that be appropriate for batman as well?
> >
> > I thought that too.
> >
> > But at this point, wouldn't it be better to do a reset here and remove the other
> > resets from any other encapsulation module?
> >
> > Maybe this operation is supposed to not happen if no encapsulation is involved?
>
> This is exactly right. The reset happens much later if there is no
> encapsulation. However, if there is an encapsuation that changes the
> hader values that are used to filter, then nf_reset has to happen.
> That is why nf_reset happens input to the encapsulation layer instead
> of always on output from bridge.
I see.
Then I think that this patch can be dropped.
I will provide another patch to batman-adv so that it can reset the nf state
before extracting the payload packet.
Thanks a lot for your feedback!
Regards,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net] bridge: clean the nf_bridge status when forwarding the skb
From: Vlad Yasevich @ 2013-10-21 15:00 UTC (permalink / raw)
To: Antonio Quartulli
Cc: Pablo Neira Ayuso, Antonio Quartulli, David S. Miller,
netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <20131018154123.GM2596@neomailbox.net>
On 10/18/2013 11:41 AM, Antonio Quartulli wrote:
> On Fri, Oct 18, 2013 at 11:33:06AM -0400, Vlad Yasevich wrote:
>> On 10/18/2013 10:46 AM, Antonio Quartulli wrote:
>>> On Fri, Oct 18, 2013 at 10:32:09AM -0400, Vlad Yasevich wrote:
>>>> On 10/18/2013 07:35 AM, Antonio Quartulli wrote:
>>>>> On Fri, Oct 18, 2013 at 01:10:41PM +0200, Pablo Neira Ayuso wrote:
>>>>>> On Thu, Oct 17, 2013 at 01:37:35PM +0200, Antonio Quartulli wrote:
>>>>>>> On Thu, Oct 17, 2013 at 04:28:57AM -0700, Pablo Neira Ayuso wrote:
>>>
>>> [...]
>>>
>>>>>
>>>>> The problem I was having was due to an skb entering br0 first and br1 later.
>>>>> When reaching br1 skb->nf_bridge was != NULL because of the previous processing
>>>>> in br0.
>>>>>
>>>>
>>>> Doesn't br_nf_pre_routing already take care of this for you? It will
>>>> drop the ref on the current nf_bridge and allocate a new one. Is that
>>>> not sufficient?
>>>
>>> In my case that line is not reached because
>>>
>>> 700 if (!IS_IP(skb) && !IS_VLAN_IP(skb) && !IS_PPPOE_IP(skb))
>>>
>>> is always true: the packet getting analysed is a batman-adv encapsulated packet,
>>> which does not match any of the three above.
>>>
>>> Cheers,
>>>
>>
>> Looking at other encapsulators (PPP, iptunnel, VXLAN), they do
>> nf_reset() on input. Would that be appropriate for batman as well?
>
> I thought that too.
>
> But at this point, wouldn't it be better to do a reset here and remove the other
> resets from any other encapsulation module?
>
> Maybe this operation is supposed to not happen if no encapsulation is involved?
This is exactly right. The reset happens much later if there is no
encapsulation. However, if there is an encapsuation that changes the
hader values that are used to filter, then nf_reset has to happen.
That is why nf_reset happens input to the encapsulation layer instead
of always on output from bridge.
-vlad
> I thought that polishing the nf state when exiting the nf related path was a
> clean and easy solution.
>
> Moreover we avoid that any newly implemented tunneling module hit this problem again.
>
>
> Cheers,
>
^ permalink raw reply
* Re: [PATCH RFC] xfrm: Don't queue retransmitted packets if the original is still on the host
From: Steffen Klassert @ 2013-10-21 14:51 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20131018.163412.487683352492277510.davem@davemloft.net>
On Fri, Oct 18, 2013 at 04:34:12PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 18 Oct 2013 13:23:45 -0700
>
> > On Fri, 2013-10-18 at 16:19 -0400, David Miller wrote:
> >> From: Steffen Klassert <steffen.klassert@secunet.com>
> >> Date: Wed, 16 Oct 2013 13:42:47 +0200
> >>
> >> > It does not make sense to queue retransmitted packets if the
> >> > original packet is still in some queue of this host. So add
> >> > a check to xdst_queue_output() and drop the packet if the
> >> > original packet is not yet sent.
> >> >
> >> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> >>
> >> I have no problems with this, what about you Eric?
> >
> > It looks fine to me.
> >
> > Acked-by: Eric Dumazet <edumazet@google.com>
>
> Great, Steffen I assume you'll merge this in via your ipsec tree.
Now applied to the ipsec-next tree. Thanks everyone for the review!
^ permalink raw reply
* Re: [PATCH ipsec-next] xfrm: use vmalloc_node() for percpu scratches
From: Steffen Klassert @ 2013-10-21 14:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Herbert Xu, David S. Miller, netdev
In-Reply-To: <1382093656.3284.14.camel@edumazet-glaptop.roam.corp.google.com>
On Fri, Oct 18, 2013 at 03:54:16AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> scratches are per cpu, we can use vmalloc_node() for proper
> NUMA affinity.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied to ipsec-next, thanks a lot Eric!
^ permalink raw reply
* Re: [PATCH 15/15] net: enic: remove unnecessary pci_set_drvdata()
From: Sujith Sankar (ssujith) @ 2013-10-21 14:02 UTC (permalink / raw)
To: Jingoo Han, 'David S. Miller'
Cc: netdev@vger.kernel.org, Christian Benvenuti (benve),
'Govindarajulu Varadarajan', Neel Patel (neepatel)
In-Reply-To: <00be01cecb98$8cabcd20$a6036760$%han@samsung.com>
This looks good to me.
Regards,
-Sujith
On 18/10/13 5:55 AM, "Jingoo Han" <jg1.han@samsung.com> wrote:
>The driver core clears the driver data to NULL after device_release
>or on probe failure. Thus, it is not needed to manually clear the
>device driver data to NULL.
>
>Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>---
> drivers/net/ethernet/cisco/enic/enic_main.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c
>b/drivers/net/ethernet/cisco/enic/enic_main.c
>index 7b756cf9..ff78dfa 100644
>--- a/drivers/net/ethernet/cisco/enic/enic_main.c
>+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
>@@ -2309,7 +2309,6 @@ err_out_release_regions:
> err_out_disable_device:
> pci_disable_device(pdev);
> err_out_free_netdev:
>- pci_set_drvdata(pdev, NULL);
> free_netdev(netdev);
>
> return err;
>@@ -2338,7 +2337,6 @@ static void enic_remove(struct pci_dev *pdev)
> enic_iounmap(enic);
> pci_release_regions(pdev);
> pci_disable_device(pdev);
>- pci_set_drvdata(pdev, NULL);
> free_netdev(netdev);
> }
> }
>--
>1.7.10.4
>
>
^ permalink raw reply
* Re: [PATCH RFC 5/5] net:stmmac: asynchronous tx_clean
From: Giuseppe CAVALLARO @ 2013-10-21 13:52 UTC (permalink / raw)
To: Jimmy Perchet; +Cc: netdev
In-Reply-To: <1381937052-8999-6-git-send-email-jimmy.perchet@parrot.com>
Hello Jimmy
On 10/16/2013 5:24 PM, Jimmy Perchet wrote:
> Tx descriptor's cleanup and preparation are serialized, which is not necessary
> and decrease performance.
> In addition TX descriptor's cleanup is performed on NET_RX softirq, this is
> confusing.
hmm, here you are changing the logic behind the tx/rx processes.
As done in many drivers, the stmmac cleans the tx resources in
NAPI context and this is not a confuse approach ;-).
It gave me some performance improvements especially on TCP benchmarks.
> This patch unserialize tx descriptor's cleanup and preparation
> and defer cleanup in workqueue.
So you decide to use workqueue and I kindly ask you to give me more
details about the performance improvements (UDP/TCP) and cpu usage.
I can try to do some tests on my side too. This could take a while
unfortunately.
peppe
>
> Signed-off-by: Jimmy Perchet <jimmy.perchet@parrot.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/chain_mode.c | 6 +--
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 6 ++-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 52 +++++++++++++----------
> 3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
> index d6ed0ce..8ab83cb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
> @@ -120,7 +120,7 @@ static void stmmac_refill_desc3(void *priv_ptr, struct dma_desc *p)
> * to keep explicit chaining in the descriptor.
> */
> p->des3 = (unsigned int)(priv->dma_rx_phy +
> - (((priv->dirty_rx) + 1) %
> + ((priv->dirty_rx + 1) %
> priv->dma_rx_size) *
> sizeof(struct dma_desc));
> }
> @@ -135,9 +135,9 @@ static void stmmac_clean_desc3(void *priv_ptr, struct dma_desc *p)
> * to keep explicit chaining in the descriptor.
> */
> p->des3 = (unsigned int)(priv->dma_tx_phy +
> - (((priv->dirty_tx + 1) %
> + ((atomic_read(&priv->dirty_tx) + 1) %
> priv->dma_tx_size) *
> - sizeof(struct dma_desc)));
> + sizeof(struct dma_desc));
> }
>
> const struct stmmac_chain_mode_ops chain_mode_ops = {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index f16a9bd..d5b8906 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -38,8 +38,8 @@ struct stmmac_priv {
> struct dma_extended_desc *dma_etx ____cacheline_aligned_in_smp;
> struct dma_desc *dma_tx;
> struct sk_buff **tx_skbuff;
> - unsigned int cur_tx;
> - unsigned int dirty_tx;
> + atomic_t cur_tx;
> + atomic_t dirty_tx;
> unsigned int dma_tx_size;
> u32 tx_count_frames;
> u32 tx_coal_frames;
> @@ -106,6 +106,8 @@ struct stmmac_priv {
> u32 adv_ts;
> int use_riwt;
> spinlock_t ptp_lock;
> + struct work_struct txclean_work;
> + struct workqueue_struct *wq;
> };
>
> extern int phyaddr;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 5873246..de614ad 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -48,6 +48,7 @@
> #include <linux/seq_file.h>
> #endif /* CONFIG_STMMAC_DEBUG_FS */
> #include <linux/net_tstamp.h>
> +#include <linux/workqueue.h>
> #include "stmmac_ptp.h"
> #include "stmmac.h"
>
> @@ -205,7 +206,8 @@ static void print_pkt(unsigned char *buf, int len)
>
> static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
> {
> - return priv->dirty_tx + priv->dma_tx_size - priv->cur_tx - 1;
> + return atomic_read(&priv->dirty_tx) + priv->dma_tx_size
> + - atomic_read(&priv->cur_tx) - 1;
> }
>
> /**
> @@ -230,7 +232,7 @@ static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
> static void stmmac_enable_eee_mode(struct stmmac_priv *priv)
> {
> /* Check and enter in LPI mode */
> - if ((priv->dirty_tx == priv->cur_tx) &&
> + if ((atomic_read(&priv->dirty_tx) == atomic_read(&priv->cur_tx)) &&
> (priv->tx_path_in_lpi_mode == false))
> priv->hw->mac->set_eee_mode(priv->ioaddr);
> }
> @@ -1118,8 +1120,8 @@ static int init_dma_desc_rings(struct net_device *dev)
> priv->tx_skbuff[i] = NULL;
> }
>
> - priv->dirty_tx = 0;
> - priv->cur_tx = 0;
> + atomic_set(&priv->dirty_tx, 0);
> + atomic_set(&priv->cur_tx, 0);
>
> stmmac_clear_descriptors(priv);
>
> @@ -1264,17 +1266,17 @@ static void stmmac_dma_operation_mode(int mtu, struct stmmac_priv *priv)
> * @priv: driver private structure
> * Description: it reclaims resources after transmission completes.
> */
> -static void stmmac_tx_clean(struct stmmac_priv *priv)
> +static void stmmac_tx_clean(struct work_struct *work)
> {
> + struct stmmac_priv *priv =
> + container_of(work, struct stmmac_priv, txclean_work);
> unsigned int txsize = priv->dma_tx_size;
>
> - spin_lock(&priv->tx_lock);
> -
> priv->xstats.tx_clean++;
>
> - while (priv->dirty_tx != priv->cur_tx) {
> + while (atomic_read(&priv->dirty_tx) != atomic_read(&priv->cur_tx)) {
> int last;
> - unsigned int entry = priv->dirty_tx % txsize;
> + unsigned int entry = atomic_read(&priv->dirty_tx) % txsize;
> struct sk_buff *skb = priv->tx_skbuff[entry];
> struct dma_desc *p;
>
> @@ -1308,7 +1310,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
> }
> if (netif_msg_tx_done(priv))
> pr_debug("%s: curr %d, dirty %d\n", __func__,
> - priv->cur_tx, priv->dirty_tx);
> + atomic_read(&priv->cur_tx),
> + atomic_read(&priv->dirty_tx));
>
> if (likely(priv->tx_skbuff_dma[entry])) {
> dma_unmap_single(priv->device,
> @@ -1326,7 +1329,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
>
> priv->hw->desc->release_tx_desc(p, priv->mode);
>
> - priv->dirty_tx++;
> + wmb();
> + atomic_inc(&priv->dirty_tx);
> }
> if (unlikely(netif_queue_stopped(priv->dev) &&
> stmmac_tx_avail(priv) > STMMAC_TX_THRESH(priv))) {
> @@ -1344,7 +1348,6 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
> stmmac_enable_eee_mode(priv);
> mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
> }
> - spin_unlock(&priv->tx_lock);
> }
>
> static inline void stmmac_enable_dma_irq(struct stmmac_priv *priv)
> @@ -1380,8 +1383,8 @@ static void stmmac_tx_err(struct stmmac_priv *priv)
> priv->hw->desc->init_tx_desc(&priv->dma_tx[i],
> priv->mode,
> (i == txsize - 1));
> - priv->dirty_tx = 0;
> - priv->cur_tx = 0;
> + atomic_set(&priv->dirty_tx, 0);
> + atomic_set(&priv->cur_tx, 0);
> priv->hw->dma->start_tx(priv->ioaddr);
>
> priv->dev->stats.tx_errors++;
> @@ -1401,12 +1404,16 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
> int status;
>
> status = priv->hw->dma->dma_interrupt(priv->ioaddr, &priv->xstats);
> - if (likely((status & handle_rx)) || (status & handle_tx)) {
> + if (likely(status & handle_rx)) {
> if (likely(napi_schedule_prep(&priv->napi))) {
> stmmac_disable_dma_irq(priv);
> __napi_schedule(&priv->napi);
> }
> }
> + if (likely(status & handle_tx))
> + queue_work(priv->wq, &priv->txclean_work);
> +
> +
> if (unlikely(status & tx_hard_error_bump_tc)) {
> /* Try to bump up the dma threshold on this failure */
> if (unlikely(tc != SF_DMA_MODE) && (tc <= 256)) {
> @@ -1596,8 +1603,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> static void stmmac_tx_timer(unsigned long data)
> {
> struct stmmac_priv *priv = (struct stmmac_priv *)data;
> -
> - stmmac_tx_clean(priv);
> + queue_work(priv->wq, &priv->txclean_work);
> }
>
> /**
> @@ -1876,7 +1882,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
> if (priv->tx_path_in_lpi_mode)
> stmmac_disable_eee_mode(priv);
>
> - first_entry = entry = priv->cur_tx % txsize;
> + first_entry = entry = atomic_read(&priv->cur_tx) % txsize;
>
> csum_insertion = (skb->ip_summed == CHECKSUM_PARTIAL);
> /* To program the descriptors according to the size of the frame */
> @@ -1944,12 +1950,13 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
> priv->hw->desc->set_tx_owner(first);
> wmb();
>
> - priv->cur_tx += nb_desc;
> + atomic_add(nb_desc, &priv->cur_tx);
>
> if (netif_msg_pktdata(priv)) {
> pr_debug("%s: curr %d dirty=%d entry=%d, first=%p, nfrags=%d",
> - __func__, (priv->cur_tx % txsize),
> - (priv->dirty_tx % txsize), entry, first, nfrags);
> + __func__, (atomic_read(&priv->cur_tx) % txsize),
> + (atomic_read(&priv->dirty_tx) % txsize), entry,
> + first, nfrags);
>
> if (priv->extend_desc)
> stmmac_display_ring((void *)priv->dma_etx, txsize, 1);
> @@ -2171,7 +2178,6 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> int work_done = 0;
>
> priv->xstats.napi_poll++;
> - stmmac_tx_clean(priv);
>
> work_done = stmmac_rx(priv, budget);
> if (work_done < budget) {
> @@ -2747,6 +2753,8 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>
> spin_lock_init(&priv->lock);
> spin_lock_init(&priv->tx_lock);
> + priv->wq = create_singlethread_workqueue("stmmac_wq");
> + INIT_WORK(&priv->txclean_work, stmmac_tx_clean);
>
> ret = register_netdev(ndev);
> if (ret) {
>
^ permalink raw reply
* Re: [PATCH RFC 1/5] net:stmmac: set threshold/store and forward mode according to mtu size.
From: Giuseppe CAVALLARO @ 2013-10-21 13:49 UTC (permalink / raw)
To: Rayagond K; +Cc: Jimmy Perchet, netdev
In-Reply-To: <CAJ3bTp62KZVK22b3M5n03Bj20auaUFYDJk-_-U=cDxb2x9dFVQ@mail.gmail.com>
On 10/21/2013 11:58 AM, Rayagond K wrote:
> On Mon, Oct 21, 2013 at 2:17 PM, Giuseppe CAVALLARO
> <peppe.cavallaro@st.com> wrote:
>> >On 10/16/2013 5:24 PM, Jimmy Perchet wrote:
>>> >>
>>> >>Threshold mode dma is needed on rx path if jumbo frames are expected.
>> >
>> >
>> >I think we should not force the threshold mode depending on the mtu.
> I remember SNPS HW team suggesting me to program the rx path in
> Threshold mode for jumbo frames, as it takes care of low RX FIFO size.
>
ok. no problem to keep it as default in the stmmac.
Cheers
Peppe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox