* Re: [PATCH net-next] ixgbe: flush when in xmit_more mode and under descriptor pressure
From: Alexander Duyck @ 2014-08-26 16:24 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <53FCB400.70705@redhat.com>
On 08/26/2014 09:21 AM, Daniel Borkmann wrote:
> On 08/26/2014 06:01 PM, Alexander Duyck wrote:
> ...
>> My thought is to just fold ixgbe_maybe_stop_tx into the if statement.
>>
>> So it shoudl be:
>> if (!skb->smit_more || ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED))
>
> Right, that's better; I was just thinking about the DMA error case,
> but in that case we release resources back anyway.
Actually the order does need to be reversed though. We should test for
stop_tx first, then xmit_more. Doing it the other way around would
cause issues as maybe_stop_tx has some other side effects.
Thaks,
Alex
^ permalink raw reply
* Re: [PATCH net-next] ixgbe: flush when in xmit_more mode and under descriptor pressure
From: Daniel Borkmann @ 2014-08-26 16:21 UTC (permalink / raw)
To: Alexander Duyck; +Cc: davem, netdev
In-Reply-To: <53FCAF6C.2070401@intel.com>
On 08/26/2014 06:01 PM, Alexander Duyck wrote:
...
> My thought is to just fold ixgbe_maybe_stop_tx into the if statement.
>
> So it shoudl be:
> if (!skb->smit_more || ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED))
Right, that's better; I was just thinking about the DMA error case,
but in that case we release resources back anyway.
^ permalink raw reply
* Re: [PATCH 2/2] ixgbe: support skb->xmit_more in netdev_ops->ndo_start_xmit()
From: Tom Herbert @ 2014-08-26 16:20 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Alexander Duyck, David Miller, Linux Netdev List,
Jamal Hadi Salim, Eric Dumazet, Jeff Kirsher, Rusty Russell,
Daniel Borkmann, brouer
In-Reply-To: <1409067657.22108.7.camel@localhost>
On Tue, Aug 26, 2014 at 8:40 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Di, 2014-08-26 at 08:00 -0700, Alexander Duyck wrote:
>> On 08/25/2014 04:35 PM, David Miller wrote:
>> >
>> > From: Daniel Borkmann <dborkman@redhat.com>
>> >
>> > This implements the deferred tail pointer flush API for the ixgbe
>> > driver. Similar version also proposed longer time ago by Alexander Duyck.
>> >
>> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> > Signed-off-by: David S. Miller <davem@davemloft.net>
>> > ---
>> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
>> > 1 file changed, 4 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > index 87bd53f..ba9ceaa 100644
>> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> > @@ -6958,9 +6958,10 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
>> >
>> > tx_ring->next_to_use = i;
>> >
>> > - /* notify HW of packet */
>> > - ixgbe_write_tail(tx_ring, i);
>> > -
>> > + if (!skb->xmit_more) {
>> > + /* notify HW of packet */
>> > + ixgbe_write_tail(tx_ring, i);
>> > + }
>> > return;
>> > dma_error:
>> > dev_err(tx_ring->dev, "TX DMA map failed\n");
>> >
>>
>> It might help to add some handling for the case where xmit_more is set,
>> but the ring has become full. This current implementation introduces
>> the risk of triggering a Tx hang.
>
> IMHO this should be done before the patch lands in the driver.
>
>> My advice would be to pull the ixgbe_maybe_stop_tx code at the end of
>> xmit_frame into the if check here, and perhaps look into adding an
>> additional check to see if BQL has stopped the ring as well.
>
> I would like to have the BQL check not in the driver but in the generic
> code steering xmit_more.
>
BQL stops the queue from netdev_tx_sent_queue, we could change that to
return indication queue was stopped and use that as another check to
flush.
> Otherwise I'll like the API.
>
> David, because of debugging and driver bugs, can we have an interface
> flag for that, so we can e.g. switch xmit_more to 0 permanently? It
> would also be nice to have for documentation purposes, so ethtool e.g.
> can discover feature-set of a driver.
>
> Thanks,
> Hannes
>
>
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Thomas Graf @ 2014-08-26 16:19 UTC (permalink / raw)
To: Andy Gospodarek
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Shrijeet Mukherjee,
John Fastabend, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Jiri Pirko, Roopa Prabhu,
Jamal Hadi Salim, aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, Neil Horman, netdev,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, dborkman,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <20140826155426.GA5275-Me9pkO/C/lgvPfuUPAiksl6hYfS7NtTn@public.gmane.org>
On 08/26/14 at 11:54am, Andy Gospodarek wrote:
> It is easy to *say* it could be added later, but connecting to software
> forwarding in the kernel outside of OVS (which is important to some)
> would take significant effort since this set only connects switch
> hardware to OVS.
Can you explain why that effort is more significant if a flow API
added first? I'm not saying it is easy to offload the existing
forwarding path, otherwise it would have been done already, but
I don't understand how the proposal makes this any more difficult.
> It may be that all software-based forwarding is done via OVS in the
> future, but it feels like we are long way from that future for those
> that do not want to use an external controller.
Wait... I don't want to use OpenFlow to configure my laptop ;-)
We should leave the controller out of this discussion though. A
controller is not required to run OVS at all. OpenStack Neutron
is a very good example for that. There are even applications which
use the OVS kernel datapath but not the OVS user space portion.
We have a wide set of APIs serving different purposes and need to
account for all of them. I'm as much interested in an offloaded
nftables and tc command as you.
^ permalink raw reply
* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Michael Tokarev @ 2014-08-26 16:15 UTC (permalink / raw)
To: Arend van Spriel; +Cc: brcm80211-dev-list, linux-wireless, netdev
In-Reply-To: <53FCA001.6090404@broadcom.com>
26.08.2014 18:56, Arend van Spriel wrote:
> On 08/26/14 07:13, Michael Tokarev wrote:
[]
>> So, I'm curious -- why this driver is found in regular section of kernel?
>> It looks like it should be in staging at least, or better yet, in a new section
>> titled "toy drivers" or something like this. Does it work for any device at
>> all?
>
> Hi Michael,
>
> Thanks for your encouraging suggestions.
You might be a good politician :) These are blames, quite strong...
Seriously. As you revisited this thread, I had quite some issues and
money loss after all this, -- after initially especially choosing a
model with a supported wifi card which turned out to be non-working,
which I tried to fix by buying a new wifi NIC (which didn't work
thanks to Lenovo), and which I finally broke due to usb wifi dongle
I used.
But that's all "poetry", the main source of my blames is you reaction -
your first reply was really ecoraging, I hoped we can have some solution
together (I have strong programming background, just not as good with
kernel and drivers as you). But after your single reply, you never
spoke again until now. That's 8 months. _That_ is where my frustration
comes from.
And the actual facts are true - by now I have had 2 entirely different
devices (my original laptop which is now thrown away and another, given
by me by a friend because I needed something) - I dunno if that's
exactly the same wifi card or not (lspci output looks similar, and
this one is also bt/wifi combo), but they both doesn't work in a
quite similar way. So, the driver doesn't work with 2 out of 2 devices
which I've seen. Not a good statistics generally, but that's 100%
"hit" ratio for me. So I really am curious if this driver works at
all.
> I revisited the threads you referred to. In the boardflags it shows you
> have a bt+wifi card. It could be that is an issue. The brcmsmac supports other
> devices, ie. bcm43224/5 and the bcm4313 comes in a number of varieties.
> From what you tell here it seems you have trouble in rx path so I will look
> into that area if there is any specifically done for your variant of the bcm4313.
I tested this "new" laptop with 3.16 kernel just briefly. I may try more
complete analysis. I can participate with the debugging/dumping (I already
mentioned my strong programming background - but I'm really not into drivers
and hardware internals). So, basically, I'm all yours ;) Not that I really
need this driver anymore (since I will avoid these cards like a plague, for
obvious reasons :), but I want to have some good outcome out of all this
story :)
Thanks,
/mjt
^ permalink raw reply
* IPv6 Policy based routing not working.
From: Tushar Shinde @ 2014-08-26 16:02 UTC (permalink / raw)
To: netdev
Hello NetDev,
I am facing problem where if ipv6 route (outside to current subnet) is
reachable only if route is present in main table. Policy based routes
are not working.
Following is my setup,
eth0 inet6 2001:1::10/120 scope global
eth1 inet6 2001:1::11/120 scope global
2001:1::1 is gateway
2.6.32-431.11.2.el6.x86_64 kernel (I tried on 3.x also)
[root@ipv6node1 ~]# ip -6 route show tab 1
2001:1::/120 dev eth0 metric 1024 mtu 1500 advmss 1440 hoplimit 4294967295
default via 2001:1::1 dev eth0 metric 1024 mtu 1500 advmss 1440
hoplimit 4294967295
[root@ipv6node1 ~]#
[root@ipv6node1 ~]# ip -6 route show tab 2
2001:1::/120 dev eth1 metric 1024 mtu 1500 advmss 1440 hoplimit 4294967295
default via 2001:1::1 dev eth1 metric 1024 mtu 1500 advmss 1440
hoplimit 4294967295
[root@ipv6node1 ~]#
[root@ipv6node1 ~]# ip -6 rule show
0: from all lookup local
16383: from 2001:1::10 lookup 1
16383: from 2001:1::11 lookup 2
32766: from all lookup main
[root@ipv6node1 ~]#
If I add default vai eth0 only ip of eth0 is reachable from outside or
eth1 dont work. In above setup default route is not present.
But default entry in "table" is never getting used to resolve route
[root@ipv6node1 ~]# ping6 -I eth0 2001:2::20
connect: Network is unreachable
[root@ipv6node1 ~]#
[root@ipv6node1 ~]# ping6 -I eth1 2001:2::20
connect: Network is unreachable
[root@ipv6node1 ~]#
To dig further I added printks and found fib6_rule_match function
return 1 only for table 255 and 254 rest all tables are skipped from
search.
fib6_rule_match
if (r->src.plen) {
if (flags & RT6_LOOKUP_F_HAS_SADDR) {
if (!ipv6_prefix_equal(&fl->fl6_src, &r->src.addr,
r->src.plen)) {
printk(KERN_ALERT "TS: %s:%d ret 0\n",
__func__, __LINE__);
return 0;
}
} else if (!(r->common.flags & FIB_RULE_FIND_SADDR)) {
printk(KERN_ALERT "TS: %s:%d ret 0\n",
__func__, __LINE__);
>>> return 0;
}
}
In case of above ping code returns from FIB_RULE_FIND_SADDR check as
pointed above.
In above case flags is 1, that is RT6_LOOKUP_F_IFACE.
Do I need to pass any extra option to ping?
Same routing table works in case of IPv4 setup without any problem.
Is policy based routing supported in ipv6?
Thank you,
Tushar
^ permalink raw reply
* Re: [PATCH net-next] ixgbe: flush when in xmit_more mode and under descriptor pressure
From: Alexander Duyck @ 2014-08-26 16:01 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1409068105-16634-1-git-send-email-dborkman@redhat.com>
On 08/26/2014 08:48 AM, Daniel Borkmann wrote:
> When xmit_more mode is being used and the ring is about to become
> full, enforce a tail pointer write to the hw. Otherwise, we could
> risk a Tx hang as pointed out by Alex.
>
> Suggested-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> Hi Alex, something along that lines regarding your first comment?
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 67 +++++++++++++++------------
> 1 file changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ba9ceaa..f851e84 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6853,6 +6883,7 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
> u32 tx_flags = first->tx_flags;
> u32 cmd_type = ixgbe_tx_cmd_type(skb, tx_flags);
> u16 i = tx_ring->next_to_use;
> + bool desc_pressure;
>
> tx_desc = IXGBE_TX_DESC(tx_ring, i);
>
> @@ -6958,10 +6989,15 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
>
> tx_ring->next_to_use = i;
>
> - if (!skb->xmit_more) {
> + desc_pressure = (ixgbe_desc_unused(tx_ring) < DESC_NEEDED);
> + if (!skb->xmit_more || unlikely(desc_pressure)) {
> /* notify HW of packet */
> ixgbe_write_tail(tx_ring, i);
> +
> + if (unlikely(desc_pressure))
> + __ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
> }
> +
> return;
> dma_error:
> dev_err(tx_ring->dev, "TX DMA map failed\n");
> @@ -6978,6 +7014,7 @@ dma_error:
> }
>
> tx_ring->next_to_use = i;
> + ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
> }
>
Actually this bit here is much more complicated than it probably needs
to be.
My thought is to just fold ixgbe_maybe_stop_tx into the if statement.
So it shoudl be:
if (!skb->smit_more || ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED))
If the BQL bit is folded into the xmit_more check somewhere then that
should be enough to resolve any possible ring stalls.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 2/2] ixgbe: support skb->xmit_more in netdev_ops->ndo_start_xmit()
From: Daniel Borkmann @ 2014-08-26 15:58 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Alexander Duyck, David Miller, netdev, therbert, jhs, edumazet,
jeffrey.t.kirsher, rusty, brouer
In-Reply-To: <1409067657.22108.7.camel@localhost>
On 08/26/2014 05:40 PM, Hannes Frederic Sowa wrote:
> On Di, 2014-08-26 at 08:00 -0700, Alexander Duyck wrote:
>> On 08/25/2014 04:35 PM, David Miller wrote:
>>>
>>> From: Daniel Borkmann <dborkman@redhat.com>
>>>
>>> This implements the deferred tail pointer flush API for the ixgbe
>>> driver. Similar version also proposed longer time ago by Alexander Duyck.
>>>
>>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index 87bd53f..ba9ceaa 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -6958,9 +6958,10 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
>>>
>>> tx_ring->next_to_use = i;
>>>
>>> - /* notify HW of packet */
>>> - ixgbe_write_tail(tx_ring, i);
>>> -
>>> + if (!skb->xmit_more) {
>>> + /* notify HW of packet */
>>> + ixgbe_write_tail(tx_ring, i);
>>> + }
>>> return;
>>> dma_error:
>>> dev_err(tx_ring->dev, "TX DMA map failed\n");
>>>
>>
>> It might help to add some handling for the case where xmit_more is set,
>> but the ring has become full. This current implementation introduces
>> the risk of triggering a Tx hang.
>
> IMHO this should be done before the patch lands in the driver.
I think that xmit_more=1 should be seen as an indication/hint to the
driver so it can decide what to do with it, e.g. flush nevertheless
when it runs under pressure.
>> My advice would be to pull the ixgbe_maybe_stop_tx code at the end of
>> xmit_frame into the if check here, and perhaps look into adding an
>> additional check to see if BQL has stopped the ring as well.
>
> I would like to have the BQL check not in the driver but in the generic
> code steering xmit_more.
>
> Otherwise I'll like the API.
I agree with you that the logic when to set/unset the hint should
come from a generic core part. I think the xmit_more API seems so far
lightweight but lets see how it evolves. :) So far we don't have a
flush-only part (as before), where one could trigger that outside of
a fast-path to flush remaining skbs, but perhaps not having it is not
too bad either.
> David, because of debugging and driver bugs, can we have an interface
> flag for that, so we can e.g. switch xmit_more to 0 permanently? It
> would also be nice to have for documentation purposes, so ethtool e.g.
> can discover feature-set of a driver.
>
> Thanks,
> Hannes
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Andy Gospodarek @ 2014-08-26 15:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Shrijeet Mukherjee,
John Fastabend, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Roopa Prabhu, Jamal Hadi Salim,
aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, Neil Horman, netdev,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, dborkman,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <20140826154459.GB1843-6KJVSR23iU5sFDB2n11ItA@public.gmane.org>
On Tue, Aug 26, 2014 at 05:44:59PM +0200, Jiri Pirko wrote:
> Tue, Aug 26, 2014 at 05:29:10PM CEST, jhs-jkUAjuhPggJWk0Htik3J/w@public.gmane.org wrote:
> >On 08/26/14 11:22, Jiri Pirko wrote:
> >
> >>I do not think that really matters. Phase one is flows. After that we
> >>can focus on l2/l3. If we would be able to fit in in flows (some drivers
> >>may), then ok. If not, we extend the api with couple of more l2/l3
> >>related ndos. I see no problem there.
> >
> >Well, it matters because we are proceeding to implement L2/3.
> >i.e the simple stuff first. We dont have anything to show yet - but
> >we will hopefully have some useful bit by Plumbers.
> >So maybe best path forward is we talk then and see how we can merge
> >efforts since we cant seem to agree at this point.
>
> I think we are in agreement. We have two worlds: flows and l2/3. We need
> both for sure. And my patchset adds an initial part of the first one.
> The second one can be added later. I do not see any issues in that.
It is easy to *say* it could be added later, but connecting to software
forwarding in the kernel outside of OVS (which is important to some)
would take significant effort since this set only connects switch
hardware to OVS.
It may be that all software-based forwarding is done via OVS in the
future, but it feels like we are long way from that future for those
that do not want to use an external controller.
^ permalink raw reply
* Re: igbvf warning on 3.14.x
From: William Dauchy @ 2014-08-26 15:54 UTC (permalink / raw)
To: Fujinaka, Todd
Cc: Wyborny, Carolyn, William Dauchy, Williams, Mitch A,
netdev@vger.kernel.org, e1000-devel, Kirsher, Jeffrey T
In-Reply-To: <9B4A1B1917080E46B64F07F2989DADD6533D2F95@ORSMSX114.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
On Aug26 15:25, Fujinaka, Todd wrote:
> Can you please send clear repro steps and I'll submit it to our lab for repro. We'll need what equipment you're running (on both sides of the link), the kind of traffic you're running, the OS & kernel you're running, etc.
the switch used is a cisco C6506
the server is a dell C6100
the network card intel 82576 Gigabit Network Connection
I'm using a linux stable release v3.14.x (last version)
the boot cmdline has igb.max_vfs=2 as argument
no specific traffic but the machine is booting through pxe boot using
nfs
the trace is a bit random at boot time (some boot are clean)
here is a the trace as a reminder:
igbvf 0000:01:10.3: PF still resetting
igbvf 0000:01:10.1: PF still resetting
------------[ cut here ]------------
WARNING: CPU: 11 PID: 1 at
drivers/net/ethernet/intel/igbvf/netdev.c:1718 igbvf_close+0x66/0x70()
CPU: 11 PID: 1 Comm: swapper/0 Not tainted 3.14.17 #1
Hardware name: Dell C6100 /0D61XP, BIOS 1.69 06/22/2012
0000000000000009 ffffffffa95a5d15 0000000000000000 ffffffffa90954e8
ffff880c1d33e000 ffff880c1d33e7c0 8000000000000000 0000000000000001
0000000000000001 ffffffffa93a6676 ffff880c1d33e000 ffffc9000624bdb8
Call Trace:
[<ffffffffa95a5d15>] ? dump_stack+0x49/0x7c
[<ffffffffa90954e8>] ? warn_slowpath_common+0x68/0x80
[<ffffffffa93a6676>] ? igbvf_close+0x66/0x70
[<ffffffffa946d878>] ? __dev_close_many+0x88/0xe0
[<ffffffffa946d9f5>] ? __dev_close+0x25/0x40
[<ffffffffa947440b>] ? __dev_change_flags+0xab/0x180
[<ffffffffa94744fe>] ? dev_change_flags+0x1e/0x60
[<ffffffffa9c401fb>] ? ic_close_devs+0x2c/0x40
[<ffffffffa9c41946>] ? ip_auto_config+0xbf3/0xec7
[<ffffffffa9273890>] ? kset_register+0x50/0x70
[<ffffffffa909a2fe>] ? __insert_resource+0x1e/0x130
[<ffffffffa9c40d53>] ? root_nfs_parse_addr+0xa7/0xa7
[<ffffffffa9c10ee5>] ? do_one_initcall+0x8c/0x107
[<ffffffffa9c110ba>] ? kernel_init_freeable+0x15a/0x1e1
[<ffffffffa9c10857>] ? do_early_param+0x9b/0x9b
[<ffffffffa959e9f0>] ? rest_init+0x80/0x80
[<ffffffffa959e9f5>] ? kernel_init+0x5/0xf0
[<ffffffffa95ac764>] ? ret_from_fork+0x74/0xa0
[<ffffffffa959e9f0>] ? rest_init+0x80/0x80
---[ end trace 90b2a2646f143695 ]---
igbvf 0000:01:10.1: PF still resetting
igbvf 0000:01:10.3: PF still resetting
ixgbe 0000:03:00.0: removed PHC on eth6
ixgbe 0000:03:00.1: removed PHC on eth7
Regards,
--
William
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH net-next] ixgbe: flush when in xmit_more mode and under descriptor pressure
From: Daniel Borkmann @ 2014-08-26 15:48 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: davem, netdev
When xmit_more mode is being used and the ring is about to become
full, enforce a tail pointer write to the hw. Otherwise, we could
risk a Tx hang as pointed out by Alex.
Suggested-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
Hi Alex, something along that lines regarding your first comment?
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 67 +++++++++++++++------------
1 file changed, 38 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ba9ceaa..f851e84 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6837,6 +6837,36 @@ static void ixgbe_tx_olinfo_status(union ixgbe_adv_tx_desc *tx_desc,
tx_desc->read.olinfo_status = cpu_to_le32(olinfo_status);
}
+static int __ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
+{
+ netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
+
+ /* Herbert's original patch had:
+ * smp_mb__after_netif_stop_queue();
+ * but since that doesn't exist yet, just open code it.
+ */
+ smp_mb();
+
+ /* We need to check again in a case another CPU has just
+ * made room available.
+ */
+ if (likely(ixgbe_desc_unused(tx_ring) < size))
+ return -EBUSY;
+
+ /* A reprieve! - use start_queue because it doesn't call schedule */
+ netif_start_subqueue(tx_ring->netdev, tx_ring->queue_index);
+ ++tx_ring->tx_stats.restart_queue;
+ return 0;
+}
+
+static inline int ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
+{
+ if (likely(ixgbe_desc_unused(tx_ring) >= size))
+ return 0;
+
+ return __ixgbe_maybe_stop_tx(tx_ring, size);
+}
+
#define IXGBE_TXD_CMD (IXGBE_TXD_CMD_EOP | \
IXGBE_TXD_CMD_RS)
@@ -6853,6 +6883,7 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
u32 tx_flags = first->tx_flags;
u32 cmd_type = ixgbe_tx_cmd_type(skb, tx_flags);
u16 i = tx_ring->next_to_use;
+ bool desc_pressure;
tx_desc = IXGBE_TX_DESC(tx_ring, i);
@@ -6958,10 +6989,15 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
tx_ring->next_to_use = i;
- if (!skb->xmit_more) {
+ desc_pressure = (ixgbe_desc_unused(tx_ring) < DESC_NEEDED);
+ if (!skb->xmit_more || unlikely(desc_pressure)) {
/* notify HW of packet */
ixgbe_write_tail(tx_ring, i);
+
+ if (unlikely(desc_pressure))
+ __ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
}
+
return;
dma_error:
dev_err(tx_ring->dev, "TX DMA map failed\n");
@@ -6978,6 +7014,7 @@ dma_error:
}
tx_ring->next_to_use = i;
+ ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
}
static void ixgbe_atr(struct ixgbe_ring *ring,
@@ -7068,32 +7105,6 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
input, common, ring->queue_index);
}
-static int __ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
-{
- netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
- /* Herbert's original patch had:
- * smp_mb__after_netif_stop_queue();
- * but since that doesn't exist yet, just open code it. */
- smp_mb();
-
- /* We need to check again in a case another CPU has just
- * made room available. */
- if (likely(ixgbe_desc_unused(tx_ring) < size))
- return -EBUSY;
-
- /* A reprieve! - use start_queue because it doesn't call schedule */
- netif_start_subqueue(tx_ring->netdev, tx_ring->queue_index);
- ++tx_ring->tx_stats.restart_queue;
- return 0;
-}
-
-static inline int ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
-{
- if (likely(ixgbe_desc_unused(tx_ring) >= size))
- return 0;
- return __ixgbe_maybe_stop_tx(tx_ring, size);
-}
-
static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb,
void *accel_priv, select_queue_fallback_t fallback)
{
@@ -7262,8 +7273,6 @@ xmit_fcoe:
#endif /* IXGBE_FCOE */
ixgbe_tx_map(tx_ring, first, hdr_len);
- ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
-
return NETDEV_TX_OK;
out_drop:
--
1.7.11.7
^ permalink raw reply related
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jiri Pirko @ 2014-08-26 15:44 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Roopa Prabhu, Thomas Graf, John Fastabend, Scott Feldman, netdev,
David Miller, Neil Horman, Andy Gospodarek, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, f.fainelli, linville,
dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram
In-Reply-To: <53FCA7C6.5070804@mojatatu.com>
Tue, Aug 26, 2014 at 05:29:10PM CEST, jhs@mojatatu.com wrote:
>On 08/26/14 11:22, Jiri Pirko wrote:
>
>>I do not think that really matters. Phase one is flows. After that we
>>can focus on l2/l3. If we would be able to fit in in flows (some drivers
>>may), then ok. If not, we extend the api with couple of more l2/l3
>>related ndos. I see no problem there.
>
>Well, it matters because we are proceeding to implement L2/3.
>i.e the simple stuff first. We dont have anything to show yet - but
>we will hopefully have some useful bit by Plumbers.
>So maybe best path forward is we talk then and see how we can merge
>efforts since we cant seem to agree at this point.
I think we are in agreement. We have two worlds: flows and l2/3. We need
both for sure. And my patchset adds an initial part of the first one.
The second one can be added later. I do not see any issues in that.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: Functions to report space available in device TX queues
From: Tom Herbert @ 2014-08-26 15:44 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <20140825.173325.723351409734098574.davem@davemloft.net>
On Mon, Aug 25, 2014 at 5:33 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Mon, 25 Aug 2014 08:27:56 -0700 (PDT)
>
>> This patch adds netdev_tx_avail_queue and netdev_avail_queue which are
>> used to report number of bytes available in transmit queues per BQL. The
>> functions call dql_avail which returns BQL limit minus number of
>> inflight bytes. These functions can be called without txlock, for
>> instance to ascertain how much data should be dequeued from a qdisc in
>> a batch. When called without the tx_lock, the result is technically a
>> hint, subsequently when the tx_lock is done for a transmit it is
>> possible the availability has changed (for example a transmit
>> completion may have freed up more space in the queue or changed the
>> limit).
>>
>> Signed-off-by: Tom Herbert <therbert@google.com>
>
> Ok this looks fine, but could you please resubmit this alongside
> the first use case?
>
It should be called when determining the best size for a transmit
batch, e.g. how many bytes to dequeue from qdisc on wakeup.
> Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] ixgbe: support skb->xmit_more in netdev_ops->ndo_start_xmit()
From: Hannes Frederic Sowa @ 2014-08-26 15:40 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, netdev, therbert, jhs, edumazet, jeffrey.t.kirsher,
rusty, dborkman, brouer
In-Reply-To: <53FCA0FA.20600@intel.com>
On Di, 2014-08-26 at 08:00 -0700, Alexander Duyck wrote:
> On 08/25/2014 04:35 PM, David Miller wrote:
> >
> > From: Daniel Borkmann <dborkman@redhat.com>
> >
> > This implements the deferred tail pointer flush API for the ixgbe
> > driver. Similar version also proposed longer time ago by Alexander Duyck.
> >
> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 87bd53f..ba9ceaa 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -6958,9 +6958,10 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
> >
> > tx_ring->next_to_use = i;
> >
> > - /* notify HW of packet */
> > - ixgbe_write_tail(tx_ring, i);
> > -
> > + if (!skb->xmit_more) {
> > + /* notify HW of packet */
> > + ixgbe_write_tail(tx_ring, i);
> > + }
> > return;
> > dma_error:
> > dev_err(tx_ring->dev, "TX DMA map failed\n");
> >
>
> It might help to add some handling for the case where xmit_more is set,
> but the ring has become full. This current implementation introduces
> the risk of triggering a Tx hang.
IMHO this should be done before the patch lands in the driver.
> My advice would be to pull the ixgbe_maybe_stop_tx code at the end of
> xmit_frame into the if check here, and perhaps look into adding an
> additional check to see if BQL has stopped the ring as well.
I would like to have the BQL check not in the driver but in the generic
code steering xmit_more.
Otherwise I'll like the API.
David, because of debugging and driver bugs, can we have an interface
flag for that, so we can e.g. switch xmit_more to 0 permanently? It
would also be nice to have for documentation purposes, so ethtool e.g.
can discover feature-set of a driver.
Thanks,
Hannes
^ permalink raw reply
* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Pankaj Gupta @ 2014-08-26 15:30 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, Jiri Pirko, linux-kernel, netdev, davem,
dgibson, vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul,
therbert, bhutchings, xii, stephen
In-Reply-To: <53FAA607.3060307@redhat.com>
> On 08/24/2014 07:14 PM, Michael S. Tsirkin wrote:
> > On Fri, Aug 22, 2014 at 07:52:22AM -0400, Pankaj Gupta wrote:
> >>> On 08/20/2014 07:17 PM, Michael S. Tsirkin wrote:
> >>>> On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
> >>>>>> Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> >>>>>>>> This patch publishes maximum number of tun/tap queues allocated as a
> >>>>>>>> read_only module parameter which a user space application like
> >>>>>>>> libvirt
> >>>>>>>> can make use of to limit maximum number of queues. Value of
> >>>>>>>> read_only
> >>>>>>>> module parameter can be writable only at module load time. If no
> >>>>>>>> value is set
> >>>>>>>> at module load time a default value 256 is used which is equal to
> >>>>>>>> maximum number
> >>>>>>>> of vCPUS allowed by KVM.
> >>>>>>>>
> >>>>>>>> Administrator can specify maximum number of queues only at the
> >>>>>>>> driver
> >>>>>>>> module load time.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >>>>>>>> ---
> >>>>>>>> drivers/net/tun.c | 13 +++++++++++--
> >>>>>>>> 1 files changed, 11 insertions(+), 2 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>>>>>> index acaaf67..1f518e2 100644
> >>>>>>>> --- a/drivers/net/tun.c
> >>>>>>>> +++ b/drivers/net/tun.c
> >>>>>>>> @@ -119,6 +119,9 @@ struct tap_filter {
> >>>>>>>>
> >>>>>>>> #define TUN_FLOW_EXPIRE (3 * HZ)
> >>>>>>>>
> >>>>>>>> +static int max_tap_queues = MAX_TAP_QUEUES;
> >>>>>>>> +module_param(max_tap_queues, int, S_IRUGO);
> >>>>>> Please do not introduce new module paramaters. Please other ways to
> >>>>>> interchange values with userspace.
> >>>> I suggested this initially, but thinking more about it, I agree.
> >>>>
> >>>> It's a global limit (necessary to limit memory utilization by
> >>>> userspace), but it should be possible to change it
> >>>> after module load.
> >>> How about pass this limit through ifr during TUNSETIFF, then
> >>> alloc_netdev_mq() can use this limit.
> >> Any other ideas/comments from the experts. Or shall I re-repost other
> >> patches
> >> in the series except this patch until we agree on one.
> >>
> > It's kind of useless without a way for userspace to discover
> > how many queues it can create, no?
> >
>
> We can implement ethtool_get_channels for tuntap. But I'm still not
> clear why this is necessary.
ethtool_get_channels for tuntap sounds good idea to retrieve number of queues
configured.
>
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jamal Hadi Salim @ 2014-08-26 15:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Shrijeet Mukherjee,
John Fastabend, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Roopa Prabhu,
aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, Neil Horman, netdev,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, dborkman,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <20140826152217.GA1843-6KJVSR23iU5sFDB2n11ItA@public.gmane.org>
On 08/26/14 11:22, Jiri Pirko wrote:
> I do not think that really matters. Phase one is flows. After that we
> can focus on l2/l3. If we would be able to fit in in flows (some drivers
> may), then ok. If not, we extend the api with couple of more l2/l3
> related ndos. I see no problem there.
Well, it matters because we are proceeding to implement L2/3.
i.e the simple stuff first. We dont have anything to show yet - but
we will hopefully have some useful bit by Plumbers.
So maybe best path forward is we talk then and see how we can merge
efforts since we cant seem to agree at this point.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] net: rds: Don't allocate rds_sock on stack
From: David Miller @ 2014-08-26 15:29 UTC (permalink / raw)
To: broonie; +Cc: chien.yen, rds-devel, netdev, linaro-kernel
In-Reply-To: <20140826065409.GT17528@sirena.org.uk>
From: Mark Brown <broonie@kernel.org>
Date: Tue, 26 Aug 2014 07:54:09 +0100
> On Mon, Aug 25, 2014 at 12:57:40PM -0700, David Miller wrote:
>> From: Mark Brown <broonie@kernel.org>
>
>> > From: Mark Brown <broonie@linaro.org>
>
>> > struct rds_sock is rather large ausing the following warning in an ARM
>> > allmodconfig:
>
>> > net/rds/iw_rdma.c:200:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>
>> > Fix this by dynamically allocating struct rds_sock in rds_iw_update_cm_id
>> > instead of allocating it on the stack.
>
>> > Signed-off-by: Mark Brown <broonie@linaro.org>
>
>> I'd like you to fix this differently. Creating pseudo instances of
>> objects, and partially initializing it, just to satisfy an interface
>> is always a really bad sign.
>
>> Create a key structure argument for rds_iw_get_device() and initialize that
>> and pass it in instead, update the other caller similarly.
>
> I agree that the existing code looks like it could be improved even more
> but please bear in mind that I'm just looking for a clean build (we've
> got less than 20 warnings in allmodconfig including staging at the
> minute) rather than actively working on this code in particular - I've
> no ability to do more than build testing here.
I understand that, but please fix this bug properly.
^ permalink raw reply
* RE: igbvf warning on 3.14.x
From: Fujinaka, Todd @ 2014-08-26 15:25 UTC (permalink / raw)
To: Wyborny, Carolyn, William Dauchy, Williams, Mitch A
Cc: netdev@vger.kernel.org, e1000-devel, Kirsher, Jeffrey T
In-Reply-To: <9BBC4E0CF881AA4299206E2E1412B6264FBD7D4B@ORSMSX102.amr.corp.intel.com>
Sorry, I forgot about this one. And trimming replies on e1000-devel isn't going to help much because we're forced to use outlook here and the threading is abysmal.
Can you please send clear repro steps and I'll submit it to our lab for repro. We'll need what equipment you're running (on both sides of the link), the kind of traffic you're running, the OS & kernel you're running, etc. It might be easier to file a bug on sourceforge if you're able to do that.
Thanks.
Todd Fujinaka
Software Application Engineer
Networking Division (ND)
Intel Corporation
todd.fujinaka@intel.com
(503) 712-4565
-----Original Message-----
From: Wyborny, Carolyn
Sent: Tuesday, August 26, 2014 8:18 AM
To: William Dauchy; Williams, Mitch A; Fujinaka, Todd
Cc: netdev@vger.kernel.org; e1000-devel; Kirsher, Jeffrey T
Subject: RE: igbvf warning on 3.14.x
Adding Todd Fujinaka, who is customer support for igbvf. I'll assist if need be.
Thanks,
Carolyn
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of William Dauchy
> Sent: Tuesday, August 26, 2014 12:36 AM
> To: Wyborny, Carolyn; Williams, Mitch A
> Cc: William Dauchy; netdev@vger.kernel.org; e1000-devel; Kirsher, Jeffrey T
> Subject: Re: igbvf warning on 3.14.x
>
> Hi,
>
> On Aug15 06:27, Jeff Kirsher wrote:
> > Adding Carolyn (igb maintainer) and e1000-devel mailing list...
>
> any chance to get some feedback on this trace?
>
> Thanks,
> --
> William
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jiri Pirko @ 2014-08-26 15:22 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Roopa Prabhu, Thomas Graf, John Fastabend, Scott Feldman, netdev,
David Miller, Neil Horman, Andy Gospodarek, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, f.fainelli, linville,
dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram
In-Reply-To: <53FCA0AE.9010304@mojatatu.com>
Tue, Aug 26, 2014 at 04:58:54PM CEST, jhs@mojatatu.com wrote:
>On 08/26/14 10:06, Jiri Pirko wrote:
>
>>Yes. Flows are phase one. The api will be extended in for whatever is
>>needed for l2/l3 as you said. Also I see a possibility to implement the
>>l2/l3 use case with flows as well.
>
>And as a note: This is where i have the disagreement.
>It is good there is acknowledgement you are handling flows for now.
>Or whatever tuples you defined as "flow". I dont think L2 or 3 fit
>in that. If thats not what you are saying then we are in agreement.
I do not think that really matters. Phase one is flows. After that we
can focus on l2/l3. If we would be able to fit in in flows (some drivers
may), then ok. If not, we extend the api with couple of more l2/l3
related ndos. I see no problem there.
^ permalink raw reply
* RE: igbvf warning on 3.14.x
From: Wyborny, Carolyn @ 2014-08-26 15:17 UTC (permalink / raw)
To: William Dauchy, Williams, Mitch A, Fujinaka, Todd
Cc: netdev@vger.kernel.org, e1000-devel, Kirsher, Jeffrey T
In-Reply-To: <20140826073613.GH8801@gandi.net>
Adding Todd Fujinaka, who is customer support for igbvf. I'll assist if need be.
Thanks,
Carolyn
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of William Dauchy
> Sent: Tuesday, August 26, 2014 12:36 AM
> To: Wyborny, Carolyn; Williams, Mitch A
> Cc: William Dauchy; netdev@vger.kernel.org; e1000-devel; Kirsher, Jeffrey T
> Subject: Re: igbvf warning on 3.14.x
>
> Hi,
>
> On Aug15 06:27, Jeff Kirsher wrote:
> > Adding Carolyn (igb maintainer) and e1000-devel mailing list...
>
> any chance to get some feedback on this trace?
>
> Thanks,
> --
> William
^ permalink raw reply
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Jamal Hadi Salim @ 2014-08-26 15:12 UTC (permalink / raw)
To: Scott Feldman, Roopa Prabhu
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Shrijeet Mukherjee,
John Fastabend, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw, Jiri Pirko,
aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, Neil Horman, netdev,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, dborkman,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, David Miller
In-Reply-To: <D891A8EC-548C-453E-AC70-8431DAC4B8C4-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
On 08/26/14 11:01, Scott Feldman wrote:
>
>
> I don’t see it that way. I believe sw_flow can be the intermediary representation to span flow-based and non-flow-based HW,
> and from flow-based world and traditional l2/l3 world.
>
>
Is there more magic to this than what Thomas just presented in this thread?
cheers,
jamal
^ permalink raw reply
* [PATCH v3 net-next 2/2] tipc: add name distributor resiliency queue
From: erik.hugne @ 2014-08-26 15:05 UTC (permalink / raw)
To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne
In-Reply-To: <1409065550-9648-1-git-send-email-erik.hugne@ericsson.com>
From: Erik Hugne <erik.hugne@ericsson.com>
TIPC name table updates are distributed asynchronously in a cluster,
entailing a risk of certain race conditions. E.g., if two nodes
simultaneously issue conflicting (overlapping) publications, this may
not be detected until both publications have reached a third node, in
which case one of the publications will be silently dropped on that
node. Hence, we end up with an inconsistent name table.
In most cases this conflict is just a temporary race, e.g., one
node is issuing a publication under the assumption that a previous,
conflicting, publication has already been withdrawn by the other node.
However, because of the (rtt related) distributed update delay, this
may not yet hold true on all nodes. The symptom of this failure is a
syslog message: "tipc: Cannot publish {%u,%u,%u}, overlap error".
In this commit we add a resiliency queue at the receiving end of
the name table distributor. When insertion of an arriving publication
fails, we retain it in this queue for a short amount of time, assuming
that another update will arrive very soon and clear the conflict. If so
happens, we insert the publication, otherwise we drop it.
The (configurable) retention value defaults to 2000 ms. Knowing from
experience that the situation described above is extremely rare, there
is no risk that the queue will accumulate any large number of items.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
---
v3: Use time_after() when comparing jiffy time
v2: Fixed phrasing and spelling in the sysctl documentation
Documentation/sysctl/net.txt | 16 ++++++++++
net/tipc/core.h | 1 +
net/tipc/name_distr.c | 69 ++++++++++++++++++++++++++++++++++++++++++--
net/tipc/name_distr.h | 1 +
net/tipc/name_table.c | 8 ++---
net/tipc/sysctl.c | 7 +++++
6 files changed, 95 insertions(+), 7 deletions(-)
diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
index 9a0319a..04892b8 100644
--- a/Documentation/sysctl/net.txt
+++ b/Documentation/sysctl/net.txt
@@ -241,6 +241,9 @@ address of the router (or Connected) for internal networks.
6. TIPC
-------------------------------------------------------
+tipc_rmem
+----------
+
The TIPC protocol now has a tunable for the receive memory, similar to the
tcp_rmem - i.e. a vector of 3 INTEGERs: (min, default, max)
@@ -252,3 +255,16 @@ The max value is set to CONN_OVERLOAD_LIMIT, and the default and min values
are scaled (shifted) versions of that same value. Note that the min value
is not at this point in time used in any meaningful way, but the triplet is
preserved in order to be consistent with things like tcp_rmem.
+
+named_timeout
+--------------
+
+TIPC name table updates are distributed asynchronously in a cluster, without
+any form of transaction handling. This means that different race scenarios are
+possible. One such is that a name withdrawal sent out by one node and received
+by another node may arrive after a second, overlapping name publication already
+has been accepted from a third node, although the conflicting updates
+originally may have been issued in the correct sequential order.
+If named_timeout is nonzero, failed topology updates will be placed on a defer
+queue until another event arrives that clears the error, or until the timeout
+expires. Value is in milliseconds.
diff --git a/net/tipc/core.h b/net/tipc/core.h
index d2607a8..f773b14 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -81,6 +81,7 @@ extern u32 tipc_own_addr __read_mostly;
extern int tipc_max_ports __read_mostly;
extern int tipc_net_id __read_mostly;
extern int sysctl_tipc_rmem[3] __read_mostly;
+extern int sysctl_tipc_named_timeout __read_mostly;
/*
* Other global variables
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 0591f33..82a62e5 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -1,7 +1,7 @@
/*
* net/tipc/name_distr.c: TIPC name distribution code
*
- * Copyright (c) 2000-2006, Ericsson AB
+ * Copyright (c) 2000-2006, 2014, Ericsson AB
* Copyright (c) 2005, 2010-2011, Wind River Systems
* All rights reserved.
*
@@ -71,6 +71,21 @@ static struct publ_list *publ_lists[] = {
};
+int sysctl_tipc_named_timeout __read_mostly = 2000;
+
+/**
+ * struct tipc_dist_queue - queue holding deferred name table updates
+ */
+static struct list_head tipc_dist_queue = LIST_HEAD_INIT(tipc_dist_queue);
+
+struct distr_queue_item {
+ struct distr_item i;
+ u32 dtype;
+ u32 node;
+ u64 expiry;
+ struct list_head next;
+};
+
/**
* publ_to_item - add publication info to a publication message
*/
@@ -299,6 +314,52 @@ struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
}
/**
+ * tipc_named_add_backlog - add a failed name table update to the backlog
+ *
+ */
+static void tipc_named_add_backlog(struct distr_item *i, u32 type, u32 node)
+{
+ struct distr_queue_item *e;
+ u64 now = get_jiffies_64();
+
+ e = kzalloc(sizeof(*e), GFP_ATOMIC);
+ if (!e)
+ return;
+ e->dtype = type;
+ e->node = node;
+ e->expiry = now + msecs_to_jiffies(sysctl_tipc_named_timeout);
+ memcpy(e, i, sizeof(*i));
+ list_add_tail(&e->next, &tipc_dist_queue);
+}
+
+/**
+ * tipc_named_process_backlog - try to process any pending name table updates
+ * from the network.
+ */
+void tipc_named_process_backlog(void)
+{
+ struct distr_queue_item *e, *tmp;
+ char addr[16];
+ u64 now = get_jiffies_64();
+
+ list_for_each_entry_safe(e, tmp, &tipc_dist_queue, next) {
+ if (time_after(e->expiry, now)) {
+ if (!tipc_update_nametbl(&e->i, e->node, e->dtype))
+ continue;
+ } else {
+ tipc_addr_string_fill(addr, e->node);
+ pr_warn_ratelimited("Dropping name table update (%d) of {%u, %u, %u} from %s key=%u\n",
+ e->dtype, ntohl(e->i.type),
+ ntohl(e->i.lower),
+ ntohl(e->i.upper),
+ addr, ntohl(e->i.key));
+ }
+ list_del(&e->next);
+ kfree(e);
+ }
+}
+
+/**
* tipc_named_rcv - process name table update message sent by another node
*/
void tipc_named_rcv(struct sk_buff *buf)
@@ -306,13 +367,15 @@ void tipc_named_rcv(struct sk_buff *buf)
struct tipc_msg *msg = buf_msg(buf);
struct distr_item *item = (struct distr_item *)msg_data(msg);
u32 count = msg_data_sz(msg) / ITEM_SIZE;
+ u32 node = msg_orignode(msg);
write_lock_bh(&tipc_nametbl_lock);
while (count--) {
- tipc_update_nametbl(item, msg_orignode(msg),
- msg_type(msg));
+ if (!tipc_update_nametbl(item, node, msg_type(msg)))
+ tipc_named_add_backlog(item, msg_type(msg), node);
item++;
}
+ tipc_named_process_backlog();
write_unlock_bh(&tipc_nametbl_lock);
kfree_skb(buf);
}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index 8afe32b..b9e75fe 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -73,5 +73,6 @@ void named_cluster_distribute(struct sk_buff *buf);
void tipc_named_node_up(u32 dnode);
void tipc_named_rcv(struct sk_buff *buf);
void tipc_named_reinit(void);
+void tipc_named_process_backlog(void);
#endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c058e30..3a6a0a7 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -261,8 +261,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
/* Lower end overlaps existing entry => need an exact match */
if ((sseq->lower != lower) || (sseq->upper != upper)) {
- pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
- type, lower, upper);
return NULL;
}
@@ -284,8 +282,6 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
/* Fail if upper end overlaps into an existing entry */
if ((inspos < nseq->first_free) &&
(upper >= nseq->sseqs[inspos].lower)) {
- pr_warn("Cannot publish {%u,%u,%u}, overlap error\n",
- type, lower, upper);
return NULL;
}
@@ -677,6 +673,8 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
if (likely(publ)) {
table.local_publ_count++;
buf = tipc_named_publish(publ);
+ /* Any pending external events? */
+ tipc_named_process_backlog();
}
write_unlock_bh(&tipc_nametbl_lock);
@@ -698,6 +696,8 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
if (likely(publ)) {
table.local_publ_count--;
buf = tipc_named_withdraw(publ);
+ /* Any pending external events? */
+ tipc_named_process_backlog();
write_unlock_bh(&tipc_nametbl_lock);
list_del_init(&publ->pport_list);
kfree(publ);
diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c
index f3fef93..1a779b1 100644
--- a/net/tipc/sysctl.c
+++ b/net/tipc/sysctl.c
@@ -47,6 +47,13 @@ static struct ctl_table tipc_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "named_timeout",
+ .data = &sysctl_tipc_named_timeout,
+ .maxlen = sizeof(sysctl_tipc_named_timeout),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{}
};
--
1.8.3.2
^ permalink raw reply related
* [PATCH v3 net-next 1/2] tipc: refactor name table updates out of named packet receive routine
From: erik.hugne @ 2014-08-26 15:05 UTC (permalink / raw)
To: jon.maloy, ying.xue, richard.alpe, netdev; +Cc: tipc-discussion, Erik Hugne
From: Erik Hugne <erik.hugne@ericsson.com>
We need to perform the same actions when processing deferred name
table updates, so this functionality is moved to a separate
function.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
---
net/tipc/name_distr.c | 74 ++++++++++++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 36 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index dcc15bc..0591f33 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -263,52 +263,54 @@ static void named_purge_publ(struct publication *publ)
}
/**
+ * tipc_update_nametbl - try to process a nametable update and notify
+ * subscribers
+ *
+ * tipc_nametbl_lock must be held.
+ * Returns the publication item if successful, otherwise NULL.
+ */
+struct publication *tipc_update_nametbl(struct distr_item *i, u32 node,
+ u32 dtype)
+{
+ struct publication *publ = NULL;
+
+ if (dtype == PUBLICATION) {
+ publ = tipc_nametbl_insert_publ(ntohl(i->type), ntohl(i->lower),
+ ntohl(i->upper),
+ TIPC_CLUSTER_SCOPE, node,
+ ntohl(i->ref), ntohl(i->key));
+ if (publ) {
+ tipc_nodesub_subscribe(&publ->subscr, node, publ,
+ (net_ev_handler)
+ named_purge_publ);
+ }
+ } else if (dtype == WITHDRAWAL) {
+ publ = tipc_nametbl_remove_publ(ntohl(i->type), ntohl(i->lower),
+ node, ntohl(i->ref),
+ ntohl(i->key));
+ if (publ) {
+ tipc_nodesub_unsubscribe(&publ->subscr);
+ kfree(publ);
+ }
+ } else {
+ pr_warn("Unrecognized name table message received\n");
+ }
+ return publ;
+}
+
+/**
* tipc_named_rcv - process name table update message sent by another node
*/
void tipc_named_rcv(struct sk_buff *buf)
{
- struct publication *publ;
struct tipc_msg *msg = buf_msg(buf);
struct distr_item *item = (struct distr_item *)msg_data(msg);
u32 count = msg_data_sz(msg) / ITEM_SIZE;
write_lock_bh(&tipc_nametbl_lock);
while (count--) {
- if (msg_type(msg) == PUBLICATION) {
- publ = tipc_nametbl_insert_publ(ntohl(item->type),
- ntohl(item->lower),
- ntohl(item->upper),
- TIPC_CLUSTER_SCOPE,
- msg_orignode(msg),
- ntohl(item->ref),
- ntohl(item->key));
- if (publ) {
- tipc_nodesub_subscribe(&publ->subscr,
- msg_orignode(msg),
- publ,
- (net_ev_handler)
- named_purge_publ);
- }
- } else if (msg_type(msg) == WITHDRAWAL) {
- publ = tipc_nametbl_remove_publ(ntohl(item->type),
- ntohl(item->lower),
- msg_orignode(msg),
- ntohl(item->ref),
- ntohl(item->key));
-
- if (publ) {
- tipc_nodesub_unsubscribe(&publ->subscr);
- kfree(publ);
- } else {
- pr_err("Unable to remove publication by node 0x%x\n"
- " (type=%u, lower=%u, ref=%u, key=%u)\n",
- msg_orignode(msg), ntohl(item->type),
- ntohl(item->lower), ntohl(item->ref),
- ntohl(item->key));
- }
- } else {
- pr_warn("Unrecognized name table message received\n");
- }
+ tipc_update_nametbl(item, msg_orignode(msg),
+ msg_type(msg));
item++;
}
write_unlock_bh(&tipc_nametbl_lock);
--
1.8.3.2
^ permalink raw reply related
* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: Scott Feldman @ 2014-08-26 15:01 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Thomas Graf, Jamal Hadi Salim, John Fastabend, Jiri Pirko, netdev,
David Miller, Neil Horman, Andy Gospodarek, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, f.fainelli, linville,
dev, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye
In-Reply-To: <53FC909D.8090000@cumulusnetworks.com>
On Aug 26, 2014, at 6:50 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On 8/25/14, 3:50 PM, Thomas Graf wrote:
>> On 08/25/14 at 12:15pm, Jamal Hadi Salim wrote:
>>> On 08/25/14 10:17, Thomas Graf wrote:
>>>> On 08/25/14 at 09:53am, Jamal Hadi Salim wrote:
>>>> fdb_add() *is* flow based. At least in my understanding, the whole
>>>> point here is to extend the idea of fdb_add() and make it understand
>>>> L2-L4 in a more generic way for the most common protocols.
>>>>
>>>> The reason fdb_add() is not reused is because it is Netlink specific
>>>> and only suitable for User -> HW offload. Kernel -> HW offload is
>>>> technically possible but not clean.
>>>>
>>> I dont think we have a problem handling any of this today.
>> Yes we do. It's restricted to L2 and we can't extend it easily
>> because it is based on NDA_*. The use of Netlink makes in-kernel
>> usage a pain. To me this is the sole reason for not using fdb_add()
>> in the first place. It seems absolutely clear though that fdb_add()
>> should be removed after the more generic ndo is in place providing
>> a superset of what fdb_add() can do today.
>>
>>> This is where our (shall i say strong) disagreement is.
>>> I think you will find it non-trivial to show me how you can
>>> actually take the simple L2 bridge and map it to a "flow".
>>> Since your starting point is "everything can be represented via a flow
>>> and some table" - we are at a crosspath.
>> OK, let me do the convertion for you:
>>
>> NDA_DST unused
>> NDA_LLADDR sw_flow_key.eth.dst
>> NDA_CACHEINFO unused
>> NDA_PROBES unused
>> NDA_VLAN sw_flow_key.eth.tci
>> NDA_PORT unused
>> NDA_VNI sw_flow_key.tun_key.tun_id
>> NDA_IFINDEX sw_flow_key.phys.in_port
>> NDA_MASTER unused
>>
>>> The tc filter API seems to be doing just that.
>>> You have different types of classifiers - the h/w may not be able
>>> to support some classifier types - but that is a capability discovery
>>> challenge.
>> Agreed but tc is only one out of many possible existing interfaces
>> we have. macvtap (given we want to extend beyond L2), routing,
>> OVS, bridge and eventually even things like a team device can and
>> should make use of offloads.
>>
>>> I am saying two things:
>>> 1) There are a few "fundamental" interfaces; L2 and L3 being some.
>>> Add crypto offload and a few i mentioned in my presentation. We
>> Can you share that preso? I was not present.
>>
>>> know how to do those. example; there is nothing i cant do with
>>> the rtmsg that is L3. or the fdb/port/vlan filter for L2.
>>> This flow thing should stay out of those.
>> Let me remind you about the name of the structure behind all L3
>> forwarding decisions:
>>
>> struct flowi4 {
>> [...]
>> }
>>
>> Adding a route means adding a flow. Can we please stop the flow
>> bashing? The concept of a flow is very generic, well known and already
>> very present in the kernel.
>>
>> The sw_flow_key proposed comes close to flowi4. Some fields are
>> different. They can eventually get merged. The strict IPv4/IPv6
>> separation is what makes it non obvious and probably why Jiri chose
>> the OVS representation. If you say rtmsg is complete then that clearly
>> is not the case. In particular VTEP fields, ARP, and TCP flags are
>> clearly missing for many uses.
>>
>> Again, I'm not saying flow is the ultimate answer to everything. It
>> is not. But a lot of hardware out there is aware of flows in combination
>> with some form of action execution. Non flow based hardware can have
>> their own classifier.
>>
>>> 2) The flow thing should allow a variety of classifiers to be
>>> handled. Again capability discovery would take care of differences.
>> So you want the flow to represent something that is not a flow. Again,
>> this comes back to the conversation in the other email. If this is
>> all about having a single ndo I'm sure we can find common grounds on
>> that.
>
> From what i understood (trying to summarize here for my own benefit):
> the switchdev api currently under review proposes every switch asic offload abstraction as a flow.
> It does not mandate this via code, however, there seems to be some discussion along those lines.
>
> The switchdev api flow ndo's need to stay for switch asic drivers that support flows directly or
> possibly want all their hw offload abstraction to be represented by the flow abstraction (openvswitch, the rocker dev ). The details of how the flow is mapped to hw lies in the corresponding switch driver code.
>
> We think rtnetlink is the api to model switch asic hw tables.
> We have a working model (Cumulus) that maps rtnetlink to switch
> asic hw tables (via snooping rtnetlink msgs). This can be done by extending the switchdev api
> with new ndo's for l2 and l3.
>
I don’t see it that way. I believe sw_flow can be the intermediary representation to span flow-based and non-flow-based HW, and from flow-based world and traditional l2/l3 world.
> Example:
> new switchdev ndo's for fdb_add/fdb_del
> new switchdev ndo's for l3
>
> Now we only need working patches that implement switchdev api ndo ops for l2/l3 (this is in the works).
>
> As long as the current patches under review allow the extension of the api to cover non-flow based l2/l3 switch asic offloads, we might be good (?).
>
> Thanks,
> Roopa
>
>
>
-scott
^ permalink raw reply
* Re: [PATCH 2/2] ixgbe: support skb->xmit_more in netdev_ops->ndo_start_xmit()
From: Alexander Duyck @ 2014-08-26 15:00 UTC (permalink / raw)
To: David Miller, netdev
Cc: therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty,
dborkman, brouer
In-Reply-To: <20140825.163505.1969688661687017343.davem@davemloft.net>
On 08/25/2014 04:35 PM, David Miller wrote:
>
> From: Daniel Borkmann <dborkman@redhat.com>
>
> This implements the deferred tail pointer flush API for the ixgbe
> driver. Similar version also proposed longer time ago by Alexander Duyck.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 87bd53f..ba9ceaa 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6958,9 +6958,10 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
>
> tx_ring->next_to_use = i;
>
> - /* notify HW of packet */
> - ixgbe_write_tail(tx_ring, i);
> -
> + if (!skb->xmit_more) {
> + /* notify HW of packet */
> + ixgbe_write_tail(tx_ring, i);
> + }
> return;
> dma_error:
> dev_err(tx_ring->dev, "TX DMA map failed\n");
>
It might help to add some handling for the case where xmit_more is set,
but the ring has become full. This current implementation introduces
the risk of triggering a Tx hang.
My advice would be to pull the ixgbe_maybe_stop_tx code at the end of
xmit_frame into the if check here, and perhaps look into adding an
additional check to see if BQL has stopped the ring as well.
Thanks,
Alex
^ 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