* Re: cannot ARP macvlan on top of bonding device
From: Francesco Ruggeri @ 2014-09-19 21:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Francesco Ruggeri, netdev, David Miller, herbert, nicolas.dichtel
In-Reply-To: <1411161451.26859.32.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Sep 19, 2014 at 2:17 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-09-19 at 14:00 -0700, Francesco Ruggeri wrote:
>> [Resending with correct date]
>>
>> Ping in the following test fails:
>>
>> # Create veth pair v1 v2
>> ip link add v1 type veth peer name v2
>>
>> # Stack macvlan on top of bonding device on top of v1
>> ip link set v1 up
>> ip link add bond1 type bond
>> ip link set bond1 up
>> ifenslave bond1 v1
>> ip link add link bond1 name mac1 type macvlan mode bridge
>> ip link set mac1 up
>> ifconfig mac1 1.2.3.1/24
>>
>> # Move v2 to namespace dummy2 and ping v1
>> ip netns add dummy2
>> ip link set v2 netns dummy2
>> ip netns exec dummy2 bash
>> ip link set lo up
>> ip link set v2 up
>> ifconfig v2 1.2.3.2/24
>> ping 1.2.3.1 -c 5
>>
>> # Clean up
>> exit
>> ip netns del dummy2
>> ip link del bond
>>
>> It passes on older releases.
>> The problem is that txqueuelen in bond1 is 0, and ARP requests are dropped in macvlan_broadcast_enqueue at:
>>
>> if (skb_queue_len(&port->bc_queue) < skb->dev->tx_queue_len) {
>> __skb_queue_tail(&port->bc_queue, nskb);
>> err = 0;
>> }
>>
>> where
>> port->dev = skb->dev = "bond1"
>> skb_queue_len(&port->bc_queue) = skb->dev->tx_queue_len = 0
>>
>> macvlan_broadcast_enqueue was introduced in 412ca1550 (macvlan: Move broadcasts into a work queue).
>>
>> Why are we using tx_queue_len to limit an rx queue?
>
> You're right, we probably should use netdev_max_backlog instead.
>
> Please send a formal patch ?
>
>
>
After sending my email I noticed that Nicolas Dichtel already
submitted a similar patch at
http://marc.info/?l=linux-netdev&m=141094132726614&w=2
Francesco
^ permalink raw reply
* Re: [PATCH net] macvlan: allow to enqueue broadcast pkt on virtual device
From: Eric Dumazet @ 2014-09-19 21:35 UTC (permalink / raw)
To: Cong Wang; +Cc: Nicolas Dichtel, David Miller, netdev, Herbert Xu
In-Reply-To: <CAHA+R7OQ=R8Y0LDVormukRjdosODiRuGq+NTrZQ08sXvfJYUnQ@mail.gmail.com>
On Fri, 2014-09-19 at 14:24 -0700, Cong Wang wrote:
> On Wed, Sep 17, 2014 at 1:08 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
> > Since commit 412ca1550cbe ("macvlan: Move broadcasts into a work queue"), the
> > driver uses tx_queue_len of the master device as the limit of packets enqueuing.
> > Problem is that virtual drivers have this value set to 0, thus all broadcast
> > packets were rejected.
> > Because tx_queue_len was arbitrarily chosen, I replace it with a static limit
> > of 1000 (also arbitrarily chosen).
> >
>
> Hmm, why not manually set this tx_queue_len to fix it?
>
> At least fifo qdisc also uses tx_queue_len, and when you bond
> you have to set it to non-zero.
This is not true. We usually leave virtual devices with tx_queue_len to
0, otherwise you get a qdisc on them for nothing...
Only when you want to have say HTB on the bonding, you might need to
change tx_queue_len
>
> I am not saying using this tx_queue_len is absolutely correct,
> I am saying it may be expected to set tx_queue_len by yourself.
I disagree.
^ permalink raw reply
* Re: pull-request: can 2014-09-18
From: David Miller @ 2014-09-19 21:34 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <1411036862-13255-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu, 18 Sep 2014 12:40:54 +0200
> this is a pull request of 8 patches for current net.
>
> A patch by Roger Quadros for the c_can driver fixes the swapped parameters of
> the c_can_hw_raminit_ti() function. Oliver Hartkopp adds the missing PCI ids to
> the peak_pci driver to support the single channel PCAN ExpressCard 34 adapter.
> David Dueck converts the at91_can driver to use proper clock handling
> functions. Then there are 5 patches by David Jander and me which fix several
> mailbox related problems in the flexcan driver.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH] tg3: Work around HW/FW limitations with vlan encapsulated frames
From: Vlad Yasevich @ 2014-09-19 21:33 UTC (permalink / raw)
To: Prashant Sreedharan; +Cc: Vladislav Yasevich, netdev, Michael Chan
In-Reply-To: <1411160093.6401.30.camel@prashant>
On 09/19/2014 04:54 PM, Prashant Sreedharan wrote:
> On Fri, 2014-09-19 at 09:59 -0400, Vlad Yasevich wrote:
>> On 09/18/2014 07:00 PM, Prashant Sreedharan wrote:
>>> On Thu, 2014-09-18 at 10:31 -0400, Vladislav Yasevich wrote:
>>>> TG3 appears to have an issue performing TSO and checksum offloading
>>>> correclty when the frame has been vlan encapsulated (non-accelrated).
>>>> In these cases, tcp checksum is not correctly updated.
>>>
>>> Yes that is true for inline vlan headers, to clarify was TSO and
>>> checksum offload working for accelerated 802.1ad packets ?
>>
>> We don't accelerate 802.1ad if the driver doesn't claim offload support.
>> If I had to guess, the TSO would probably work, but the packet would
>> be encapsulated as 802.1Q by the FW/HW and connection would fail.
>>
> In your initial mail you mentioned when you strip the vlan header and
> set vlan_tci it worked for you. But as you guessed it will not, as the
> chip does not support 802.1AD. Thanks for clarifying.
>
>>>
>>>> This patch attempts to work around this issue. After the patch,
>>>> 802.1ad vlans start working correctly over tg3 devices.
>>>>
>>>> CC: Prashant Sreedharan <prashant@broadcom.com>
>>>> CC: Michael Chan <mchan@broadcom.com>
>>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>>> ---
>>>> drivers/net/ethernet/broadcom/tg3.c | 20 ++++++++++++++++++--
>>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
>>>> index cb77ae9..e7d3a62 100644
>>>> --- a/drivers/net/ethernet/broadcom/tg3.c
>>>> +++ b/drivers/net/ethernet/broadcom/tg3.c
>>>> @@ -7914,8 +7914,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>
>>>> entry = tnapi->tx_prod;
>>>> base_flags = 0;
>>>> - if (skb->ip_summed == CHECKSUM_PARTIAL)
>>>> - base_flags |= TXD_FLAG_TCPUDP_CSUM;
>
> Check comment below
>>>>
>>>> mss = skb_shinfo(skb)->gso_size;
>>>> if (mss) {
>>>> @@ -7929,6 +7927,13 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>
>>>> hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb) - ETH_HLEN;
>>>>
>>>> + /* HW/FW can not correctly segment packets that have been
>>>> + * vlan encapsulated.
>>>> + */
>>>> + if (skb->protocol == htons(ETH_P_8021Q) ||
>>>> + skb->protocol == htons(ETH_P_8021AD))
>>>> + return tg3_tso_bug(tp, tnapi, txq, skb);
>>>
>>> I think skb_gso_segment() would return skbs that would still have
>>> checksum offloaded to the chip.
>>
>> Doesn't appear to. If I don't do this, then netperf throughput drops
>> to 45.75 Mbps. If I put the above back in, the throughput goes to to 940Mbps.
>>
>
> Two things either the skb is having the "encap_hdr_csum" set forcing
> skb_segment() to do the checksum calculation OR the second workaround
> "skb_checksum_help" below will do the checksum for the list of skbs
> returned by skb_gso_segment().
I traced this an it looks like checksum_help will do the checksum for segments
when tg3_start_xmit() is called for every skb on the segmented list.
>
> Also you can move the above workaround further up after skb_cow_head()
> succeeds.
>
Ok.
>>>
>>>> +
>>>> if (!skb_is_gso_v6(skb)) {
>>>> if (unlikely((ETH_HLEN + hdr_len) > 80) &&
>>>> tg3_flag(tp, TSO_BUG))
>>>> @@ -7979,6 +7984,17 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> base_flags |= tsflags << 12;
>>>> }
>>>> }
>>>> + } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>>>> + /* HW/FW can not correctly checksum packets that have been
>>>> + * vlan encapsulated.
>>>> + */
>>>> + if (skb->protocol == htons(ETH_P_8021Q) ||
>>>> + skb->protocol == htons(ETH_P_8021AD)) {
>>>> + if (skb_checksum_help(skb))
>>>> + goto drop;
>>>> + } else {
>>>> + base_flags |= TXD_FLAG_TCPUDP_CSUM;
>>>> + }
>
> tg3 supports various family of chips (around 20+ )I need to check if LSO
> offload is ok without setting TXD_FLAG_TCPUDP_CSUM for all of them and
> some of these chips are pretty old too, so instead you can clear the
> TXD_FLAG_TCPUDP_CSUM flag if skb_checksum_help() succeeds and leave the
> initial setting of base_flags as it is thus avoiding any regressions.
Hm, OK, will fix.
Thanks
-vlad
>
>>>> }
>>>>
>>>> if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
>>>
>>> Instead of the above workarounds since the chips supported by tg3 does
>>> not support checksum offload and TSO for inline vlan headers, these
>>> features can be disabled/cleared in dev->vlan_features. Side effect is
>>> accelerated vlan headers will also have TSO and checksum offload
>>> disabled.
>>
>> I didn't want to impact normal accelerated usage. The non-accelerated
>> case is rather rare especially since tg3 driver doesn't allow you to
>> turn off vlan acceleration. One has to put a software device between
>> tg3 and vlan that allow you to turn off vlan acceleration (ex: bridge)
>> and turn off acceleration on the bridge to get this to happen for 802.1Q
>> vlans. People don't normally turn off vlan acceleration though as evidenced
>> by this bug (and issues in other drivers) being around for a _very_ long time.
>>
>
> Ok, since this is rare we can go with the workaround
>>>
>>> Also as part of this review, found a problem with the receive section of
>>> the driver it was not checking for 802.1ad vlan protocol and dropping
>>> 802.1ad vlan packets of size > mtu + ETH_HLEN
>>
>> Good catch. I should have run the TCP_MAERTS test. :)
>>
>> I'll update the patch with the below hunk and resubmit if we can agree
>> on using the tg3_tso_bug workaround.
>
> yes please include. thanks.
>>
>> -vlad
>>
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/tg3.c
>>> b/drivers/net/ethernet/broadcom/tg3.c
>>> index cb77ae9..620887a 100644
>>> --- a/drivers/net/ethernet/broadcom/tg3.c
>>> +++ b/drivers/net/ethernet/broadcom/tg3.c
>>> @@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int
>>> budget)
>>> skb->protocol = eth_type_trans(skb, tp->dev);
>>>
>>> if (len > (tp->dev->mtu + ETH_HLEN) &&
>>> - skb->protocol != htons(ETH_P_8021Q)) {
>>> + skb->protocol != htons(ETH_P_8021Q) &&
>>> + skb->protocol != htons(ETH_P_8021AD)) {
>>> dev_kfree_skb_any(skb);
>>> goto drop_it_no_recycle;
>>> }
>>>
>>>
>>
>
>
^ permalink raw reply
* Re: cannot ARP macvlan on top of bonding device
From: Eric Dumazet @ 2014-09-19 21:32 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: netdev, fruggeri, davem, herbert
In-Reply-To: <1411161451.26859.32.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 2014-09-19 at 14:17 -0700, Eric Dumazet wrote:
> You're right, we probably should use netdev_max_backlog instead.
>
> Please send a formal patch ?
BTW, we should add a cond_resched() in macvlan_process_broadcast() ...
It also seems MACVLAN_SKB_CB(skb)->src escapes an RCU protected region,
without taking a refcount on struct macvlan_dev
This seems racy to me :(
^ permalink raw reply
* Re: [PATCH net-next 0/3] mlx4: CQE/EQE stride support
From: David Miller @ 2014-09-19 21:32 UTC (permalink / raw)
To: ogerlitz; +Cc: netdev, amirv, idos
In-Reply-To: <1411030261-12145-1-git-send-email-ogerlitz@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Thu, 18 Sep 2014 11:50:58 +0300
> This series from Ido Shamay is intended for archs having
> cache line larger then 64 bytes.
>
> Since our CQE/EQEs are generally 64B in those systems, HW will write
> twice to the same cache line consecutively, causing pipe locks due to
> he hazard prevention mechanism. For elements in a cyclic buffer, writes
> are consecutive, so entries smaller than a cache line should be
> avoided, especially if they are written at a high rate.
>
> Reduce consecutive writes to same cache line in CQs/EQs, by allowing the
> driver to increase the distance between entries so that each will reside
> in a different cache line.
Series applied, thanks.
^ permalink raw reply
* Re: [RFC net-next PATCH V3 0/2] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Jesper Dangaard Brouer @ 2014-09-19 21:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Tom Herbert, Hannes Frederic Sowa,
Florian Westphal, Daniel Borkmann, Jamal Hadi Salim,
Alexander Duyck, John Fastabend, Dave Taht, toke, brouer
In-Reply-To: <1411160360.26859.31.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 19 Sep 2014 13:59:20 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-09-19 at 22:49 +0200, Jesper Dangaard Brouer wrote:
> > This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
> > from the qdisc layer, to implement dequeue bulking.
> >
> > RFC V3: Keeping the ball rolling.
> >
> > This patchset should now use BQL correctly. I've done lots of testing
> > for Head-of-Line blocking issues that can occur due to requeue of a
> > SKB bulk list. I've not been able to provoke any HoL blocking
> > situation, simply because BQL is doing such a good job, thus I'm
> > unable to "overshoot" HW/BQL limits with more than a single packet.
> >
> > This patch chooses a very conservative approach, as by default only
> > allowing dequeue of one extra packet, besides the normal dequeue.
> >
> > Open questions:
> >
> > - How do we expose tuning to userspace?
> > Patch adds /proc/sys/net/core/qdisc_bulk_dequeue_limit but I don't like it...
> > Per device tunable?
> >
>
> bql is using /sys, of course ;)
>
> # grep . /sys/class/net/eth1/queues/tx-0/byte_queue_limits/*
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/hold_time:1000
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/inflight:0
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit:113314
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_max:1879048192
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_min:0
>
> Maybe you could simply reuse byte_queue_limits/limit, I am not sure we
> need a specific tunable.
It would make sense. I've been running tests with qdisc_bulk_dequeue_limit=100,
and I'm being saved/limited by the BQL limit.
Perhaps we should still keep some upper bound on num of packet e.g. 32,
as this does influence the number of HW ring descriptors we use before
"flushing"/notifying HW by the tailptr write.
>
> > - Can/should we limit dequeue bulking to devices supporting BQL?
> >
>
> Yes please. This will be an incentive to get BQL on drivers.
How can I test if the dev supports BQL?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net v2 2/2] openvswitch: restore OVS_FLOW_CMD_NEW notifications
From: David Miller @ 2014-09-19 21:29 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: pshelar, dev, netdev, samuel.gauthier
In-Reply-To: <1411029064-15376-2-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 18 Sep 2014 10:31:04 +0200
> From: Samuel Gauthier <samuel.gauthier@6wind.com>
>
> Since commit fb5d1e9e127a ("openvswitch: Build flow cmd netlink reply only if needed."),
> the new flows are not notified to the listeners of OVS_FLOW_MCGROUP.
>
> This commit fixes the problem by using the genl function, ie
> genl_has_listerners() instead of netlink_has_listeners().
>
> Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Also applied, thanks.
^ permalink raw reply
* Re: [PATCH net v2 1/2] genetlink: add function genl_has_listeners()
From: David Miller @ 2014-09-19 21:28 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: pshelar, dev, netdev
In-Reply-To: <1411029064-15376-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 18 Sep 2014 10:31:03 +0200
> This function is the counterpart of the function netlink_has_listeners().
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied.
^ permalink raw reply
* Re: [RFC net-next PATCH V3 0/2] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Dave Taht @ 2014-09-19 21:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org, David S. Miller,
Tom Herbert, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann, Jamal Hadi Salim, Alexander Duyck,
John Fastabend, Toke Høiland-Jørgensen
In-Reply-To: <1411160360.26859.31.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Sep 19, 2014 at 11:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-09-19 at 22:49 +0200, Jesper Dangaard Brouer wrote:
>> This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
>> from the qdisc layer, to implement dequeue bulking.
>>
>> RFC V3: Keeping the ball rolling.
>>
>> This patchset should now use BQL correctly. I've done lots of testing
>> for Head-of-Line blocking issues that can occur due to requeue of a
>> SKB bulk list. I've not been able to provoke any HoL blocking
>> situation, simply because BQL is doing such a good job, thus I'm
>> unable to "overshoot" HW/BQL limits with more than a single packet.
>>
>> This patch chooses a very conservative approach, as by default only
>> allowing dequeue of one extra packet, besides the normal dequeue.
>>
>> Open questions:
>>
>> - How do we expose tuning to userspace?
>> Patch adds /proc/sys/net/core/qdisc_bulk_dequeue_limit but I don't like it...
>> Per device tunable?
>>
>
> bql is using /sys, of course ;)
>
> # grep . /sys/class/net/eth1/queues/tx-0/byte_queue_limits/*
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/hold_time:1000
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/inflight:0
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit:113314
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_max:1879048192
> /sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_min:0
>
> Maybe you could simply reuse byte_queue_limits/limit, I am not sure we
> need a specific tunable.
>
>
>> - Can/should we limit dequeue bulking to devices supporting BQL?
>>
>
> Yes please. This will be an incentive to get BQL on drivers.
While I am +100 on getting more BQL'd drivers out there...
https://www.bufferbloat.net/projects/bloat/wiki/BQL_enabled_drivers
... It was my hope that some of this bulk dequeue work would apply to
wifi packet aggregation one day....
>
>
>
--
Dave Täht
https://www.bufferbloat.net/projects/make-wifi-fast
^ permalink raw reply
* Re: [PATCH net] macvlan: allow to enqueue broadcast pkt on virtual device
From: Cong Wang @ 2014-09-19 21:24 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: David Miller, netdev, Herbert Xu
In-Reply-To: <1410941288-1880-1-git-send-email-nicolas.dichtel@6wind.com>
On Wed, Sep 17, 2014 at 1:08 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> Since commit 412ca1550cbe ("macvlan: Move broadcasts into a work queue"), the
> driver uses tx_queue_len of the master device as the limit of packets enqueuing.
> Problem is that virtual drivers have this value set to 0, thus all broadcast
> packets were rejected.
> Because tx_queue_len was arbitrarily chosen, I replace it with a static limit
> of 1000 (also arbitrarily chosen).
>
Hmm, why not manually set this tx_queue_len to fix it?
At least fifo qdisc also uses tx_queue_len, and when you bond
you have to set it to non-zero.
I am not saying using this tx_queue_len is absolutely correct,
I am saying it may be expected to set tx_queue_len by yourself.
^ permalink raw reply
* Re: [PATCH net-next] net: fix sparse warnings in SNMP_UPD_PO_STATS(_BH)
From: David Miller @ 2014-09-19 21:22 UTC (permalink / raw)
To: sd; +Cc: netdev
In-Reply-To: <20140917212312.GA13866@kria>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Wed, 17 Sep 2014 23:23:12 +0200
> ptr used to be a non __percpu pointer (result of a this_cpu_ptr
> assignment, 7d720c3e4f0c4 ("percpu: add __percpu sparse annotations to
> net")). Since d25398df59b56 ("net: avoid reloads in SNMP_UPD_PO_STATS"),
> that's no longer the case, SNMP_UPD_PO_STATS uses this_cpu_add and ptr
> is now __percpu.
>
> Silence sparse warnings by preserving the original type and
> annotation, and remove the out-of-date comment.
>
> warning: incorrect type in initializer (different address spaces)
> expected unsigned long long *ptr
> got unsigned long long [noderef] <asn:3>*<noident>
> warning: incorrect type in initializer (different address spaces)
> expected void const [noderef] <asn:3>*__vpp_verify
> got unsigned long long *<noident>
> warning: incorrect type in initializer (different address spaces)
> expected void const [noderef] <asn:3>*__vpp_verify
> got unsigned long long *<noident>
>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Applied, thanks.
^ permalink raw reply
* Re: cannot ARP macvlan on top of bonding device
From: Eric Dumazet @ 2014-09-19 21:17 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: netdev, fruggeri, davem, herbert
In-Reply-To: <20140919210040.E09BA481010@fruggeri-Arora18.sjc.aristanetworks.com>
On Fri, 2014-09-19 at 14:00 -0700, Francesco Ruggeri wrote:
> [Resending with correct date]
>
> Ping in the following test fails:
>
> # Create veth pair v1 v2
> ip link add v1 type veth peer name v2
>
> # Stack macvlan on top of bonding device on top of v1
> ip link set v1 up
> ip link add bond1 type bond
> ip link set bond1 up
> ifenslave bond1 v1
> ip link add link bond1 name mac1 type macvlan mode bridge
> ip link set mac1 up
> ifconfig mac1 1.2.3.1/24
>
> # Move v2 to namespace dummy2 and ping v1
> ip netns add dummy2
> ip link set v2 netns dummy2
> ip netns exec dummy2 bash
> ip link set lo up
> ip link set v2 up
> ifconfig v2 1.2.3.2/24
> ping 1.2.3.1 -c 5
>
> # Clean up
> exit
> ip netns del dummy2
> ip link del bond
>
> It passes on older releases.
> The problem is that txqueuelen in bond1 is 0, and ARP requests are dropped in macvlan_broadcast_enqueue at:
>
> if (skb_queue_len(&port->bc_queue) < skb->dev->tx_queue_len) {
> __skb_queue_tail(&port->bc_queue, nskb);
> err = 0;
> }
>
> where
> port->dev = skb->dev = "bond1"
> skb_queue_len(&port->bc_queue) = skb->dev->tx_queue_len = 0
>
> macvlan_broadcast_enqueue was introduced in 412ca1550 (macvlan: Move broadcasts into a work queue).
>
> Why are we using tx_queue_len to limit an rx queue?
You're right, we probably should use netdev_max_backlog instead.
Please send a formal patch ?
^ permalink raw reply
* Re: [PATCH v4 net-next 0/7] net: foo-over-udp (fou)
From: David Miller @ 2014-09-19 21:16 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <1410981961-15545-1-git-send-email-therbert@google.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 17 Sep 2014 12:25:54 -0700
> This patch series implements foo-over-udp.
Series applied, thanks Tom.
^ permalink raw reply
* Re: [PATCH] tg3: Work around HW/FW limitations with vlan encapsulated frames
From: Prashant Sreedharan @ 2014-09-19 20:54 UTC (permalink / raw)
To: vyasevic; +Cc: Vladislav Yasevich, netdev, Michael Chan
In-Reply-To: <541C36CC.60405@redhat.com>
On Fri, 2014-09-19 at 09:59 -0400, Vlad Yasevich wrote:
> On 09/18/2014 07:00 PM, Prashant Sreedharan wrote:
> > On Thu, 2014-09-18 at 10:31 -0400, Vladislav Yasevich wrote:
> >> TG3 appears to have an issue performing TSO and checksum offloading
> >> correclty when the frame has been vlan encapsulated (non-accelrated).
> >> In these cases, tcp checksum is not correctly updated.
> >
> > Yes that is true for inline vlan headers, to clarify was TSO and
> > checksum offload working for accelerated 802.1ad packets ?
>
> We don't accelerate 802.1ad if the driver doesn't claim offload support.
> If I had to guess, the TSO would probably work, but the packet would
> be encapsulated as 802.1Q by the FW/HW and connection would fail.
>
In your initial mail you mentioned when you strip the vlan header and
set vlan_tci it worked for you. But as you guessed it will not, as the
chip does not support 802.1AD. Thanks for clarifying.
> >
> >> This patch attempts to work around this issue. After the patch,
> >> 802.1ad vlans start working correctly over tg3 devices.
> >>
> >> CC: Prashant Sreedharan <prashant@broadcom.com>
> >> CC: Michael Chan <mchan@broadcom.com>
> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> >> ---
> >> drivers/net/ethernet/broadcom/tg3.c | 20 ++++++++++++++++++--
> >> 1 file changed, 18 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> >> index cb77ae9..e7d3a62 100644
> >> --- a/drivers/net/ethernet/broadcom/tg3.c
> >> +++ b/drivers/net/ethernet/broadcom/tg3.c
> >> @@ -7914,8 +7914,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >>
> >> entry = tnapi->tx_prod;
> >> base_flags = 0;
> >> - if (skb->ip_summed == CHECKSUM_PARTIAL)
> >> - base_flags |= TXD_FLAG_TCPUDP_CSUM;
Check comment below
> >>
> >> mss = skb_shinfo(skb)->gso_size;
> >> if (mss) {
> >> @@ -7929,6 +7927,13 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >>
> >> hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb) - ETH_HLEN;
> >>
> >> + /* HW/FW can not correctly segment packets that have been
> >> + * vlan encapsulated.
> >> + */
> >> + if (skb->protocol == htons(ETH_P_8021Q) ||
> >> + skb->protocol == htons(ETH_P_8021AD))
> >> + return tg3_tso_bug(tp, tnapi, txq, skb);
> >
> > I think skb_gso_segment() would return skbs that would still have
> > checksum offloaded to the chip.
>
> Doesn't appear to. If I don't do this, then netperf throughput drops
> to 45.75 Mbps. If I put the above back in, the throughput goes to to 940Mbps.
>
Two things either the skb is having the "encap_hdr_csum" set forcing
skb_segment() to do the checksum calculation OR the second workaround
"skb_checksum_help" below will do the checksum for the list of skbs
returned by skb_gso_segment().
Also you can move the above workaround further up after skb_cow_head()
succeeds.
> >
> >> +
> >> if (!skb_is_gso_v6(skb)) {
> >> if (unlikely((ETH_HLEN + hdr_len) > 80) &&
> >> tg3_flag(tp, TSO_BUG))
> >> @@ -7979,6 +7984,17 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >> base_flags |= tsflags << 12;
> >> }
> >> }
> >> + } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >> + /* HW/FW can not correctly checksum packets that have been
> >> + * vlan encapsulated.
> >> + */
> >> + if (skb->protocol == htons(ETH_P_8021Q) ||
> >> + skb->protocol == htons(ETH_P_8021AD)) {
> >> + if (skb_checksum_help(skb))
> >> + goto drop;
> >> + } else {
> >> + base_flags |= TXD_FLAG_TCPUDP_CSUM;
> >> + }
tg3 supports various family of chips (around 20+ )I need to check if LSO
offload is ok without setting TXD_FLAG_TCPUDP_CSUM for all of them and
some of these chips are pretty old too, so instead you can clear the
TXD_FLAG_TCPUDP_CSUM flag if skb_checksum_help() succeeds and leave the
initial setting of base_flags as it is thus avoiding any regressions.
> >> }
> >>
> >> if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
> >
> > Instead of the above workarounds since the chips supported by tg3 does
> > not support checksum offload and TSO for inline vlan headers, these
> > features can be disabled/cleared in dev->vlan_features. Side effect is
> > accelerated vlan headers will also have TSO and checksum offload
> > disabled.
>
> I didn't want to impact normal accelerated usage. The non-accelerated
> case is rather rare especially since tg3 driver doesn't allow you to
> turn off vlan acceleration. One has to put a software device between
> tg3 and vlan that allow you to turn off vlan acceleration (ex: bridge)
> and turn off acceleration on the bridge to get this to happen for 802.1Q
> vlans. People don't normally turn off vlan acceleration though as evidenced
> by this bug (and issues in other drivers) being around for a _very_ long time.
>
Ok, since this is rare we can go with the workaround
> >
> > Also as part of this review, found a problem with the receive section of
> > the driver it was not checking for 802.1ad vlan protocol and dropping
> > 802.1ad vlan packets of size > mtu + ETH_HLEN
>
> Good catch. I should have run the TCP_MAERTS test. :)
>
> I'll update the patch with the below hunk and resubmit if we can agree
> on using the tg3_tso_bug workaround.
yes please include. thanks.
>
> -vlad
>
> >
> > diff --git a/drivers/net/ethernet/broadcom/tg3.c
> > b/drivers/net/ethernet/broadcom/tg3.c
> > index cb77ae9..620887a 100644
> > --- a/drivers/net/ethernet/broadcom/tg3.c
> > +++ b/drivers/net/ethernet/broadcom/tg3.c
> > @@ -6918,7 +6918,8 @@ static int tg3_rx(struct tg3_napi *tnapi, int
> > budget)
> > skb->protocol = eth_type_trans(skb, tp->dev);
> >
> > if (len > (tp->dev->mtu + ETH_HLEN) &&
> > - skb->protocol != htons(ETH_P_8021Q)) {
> > + skb->protocol != htons(ETH_P_8021Q) &&
> > + skb->protocol != htons(ETH_P_8021AD)) {
> > dev_kfree_skb_any(skb);
> > goto drop_it_no_recycle;
> > }
> >
> >
>
^ permalink raw reply
* Re: [PATCH 1/1 RESEND] lib: rhashtable: remove second linux/log2.h inclusion
From: David Miller @ 2014-09-19 21:09 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, netdev
In-Reply-To: <1410980839-5809-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 17 Sep 2014 21:07:19 +0200
> linux/log2.h was included twice.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/1] net: allow macvlans to move to net namespace
From: David Miller @ 2014-09-19 21:07 UTC (permalink / raw)
To: fruggeri; +Cc: netdev, fruggeri, fw, chenweilong
In-Reply-To: <20140917181227.09F4E480087@fruggeri-Arora18.sjc.aristanetworks.com>
From: fruggeri@aristanetworks.com (Francesco Ruggeri)
Date: Wed, 17 Sep 2014 11:12:27 -0700
> From 6dad8398aee7fec77838ad591871745508ed481d Mon Sep 17 00:00:00 2001
> From: Francesco Ruggeri <fruggeri@arista.com>
> Date: Wed, 17 Sep 2014 10:40:44 -0700
> Subject: [PATCH 1/1] net: allow macvlans to move to net namespace
>
> I cannot move a macvlan interface created on top of a bonding interface
> to a different namespace:
>
> % ip netns add dummy0
> % ip link add link bond0 mac0 type macvlan
> % ip link set mac0 netns dummy0
> RTNETLINK answers: Invalid argument
> %
>
> The problem seems to be that commit f9399814927a ("bonding: Don't allow
> bond devices to change network namespaces.") sets NETIF_F_NETNS_LOCAL
> on bonding interfaces, and commit 797f87f83b60 ("macvlan: fix netdev
> feature propagation from lower device") causes macvlan interfaces
> to inherit its features from the lower device.
>
> NETIF_F_NETNS_LOCAL should not be inherited from the lower device
> by a macvlan.
> Patch tested on 3.16.
>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [net-next PATCH v3] net: sched: cls_u32: rcu can not be last node
From: David Miller @ 2014-09-19 21:06 UTC (permalink / raw)
To: john.fastabend; +Cc: xiyou.wangcong, eric.dumazet, netdev, jhs
In-Reply-To: <20140917181144.9571.68528.stgit@nitbit.x32>
From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 17 Sep 2014 11:11:46 -0700
> tc_u32_sel 'sel' in tc_u_knode expects to be the last element in the
> structure and pads the structure with tc_u32_key fields for each key.
>
> kzalloc(sizeof(*n) + s->nkeys*sizeof(struct tc_u32_key), GFP_KERNEL)
>
> CC: Eric Dumazet <edumazet@google.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Applied, thanks John.
^ permalink raw reply
* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Alexei Starovoitov @ 2014-09-19 21:04 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML, Daniel Borkmann
On Thu, Sep 18, 2014 at 10:28 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/18/2014 05:24 PM, Alexei Starovoitov wrote:
> ...
>>
>> solve or not. If we decide to solve it, we need to have
>> a plan to solve it all the way. Partial fix for size of bpf_attr
>> is not a plan. It's something that is not addressing the problem
>> completely. Little bit of help is not useful for userspace. It
>> would need to deal with new types, verifier differences and
>> other things that I mentioned earlier.
>
>
> Hm, I don't think it would be a strict requirement to solve it
> all the way, and I think that perf_event_open() with perf_copy_attr()
> is not trying to do so either. It, however, is trying on a ``best
> effort basis'' to still load something if new features are unused
> by the binary (I guess you saw the comment in perf_copy_attr()).
>
> Iff, e.g. due to new types we fail at the verifier stage, sure,
> that's life since we only have backwards-compatible guarantee,
> but in case we tried to use features we support, we're still able
> to load the eBPF program while right now, we're rejecting it right
> up-front. That's just my $0.02 ...
David,
the 'changes requested' status means that you want me to
address this forward compatibility now instead of later?
Or something else?
I don't want to second guess, respin and spam people
unnecessary. In this case I don't think Daniel is insisting
on doing it in this patch set. The things discussed above
are not urgent. Unless I'm missing something.
Please clarify.
Thanks
^ permalink raw reply
* cannot ARP macvlan on top of bonding device
From: Francesco Ruggeri @ 2014-09-19 21:00 UTC (permalink / raw)
To: netdev; +Cc: fruggeri, davem, herbert
[Resending with correct date]
Ping in the following test fails:
# Create veth pair v1 v2
ip link add v1 type veth peer name v2
# Stack macvlan on top of bonding device on top of v1
ip link set v1 up
ip link add bond1 type bond
ip link set bond1 up
ifenslave bond1 v1
ip link add link bond1 name mac1 type macvlan mode bridge
ip link set mac1 up
ifconfig mac1 1.2.3.1/24
# Move v2 to namespace dummy2 and ping v1
ip netns add dummy2
ip link set v2 netns dummy2
ip netns exec dummy2 bash
ip link set lo up
ip link set v2 up
ifconfig v2 1.2.3.2/24
ping 1.2.3.1 -c 5
# Clean up
exit
ip netns del dummy2
ip link del bond
It passes on older releases.
The problem is that txqueuelen in bond1 is 0, and ARP requests are dropped in macvlan_broadcast_enqueue at:
if (skb_queue_len(&port->bc_queue) < skb->dev->tx_queue_len) {
__skb_queue_tail(&port->bc_queue, nskb);
err = 0;
}
where
port->dev = skb->dev = "bond1"
skb_queue_len(&port->bc_queue) = skb->dev->tx_queue_len = 0
macvlan_broadcast_enqueue was introduced in 412ca1550 (macvlan: Move broadcasts into a work queue).
Why are we using tx_queue_len to limit an rx queue?
Thanks,
Francesco Ruggeri
^ permalink raw reply
* Re: [RFC net-next PATCH V3 0/2] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Eric Dumazet @ 2014-09-19 20:59 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Tom Herbert, Hannes Frederic Sowa,
Florian Westphal, Daniel Borkmann, Jamal Hadi Salim,
Alexander Duyck, John Fastabend, Dave Taht, toke
In-Reply-To: <20140919204706.3231.59125.stgit@dragon>
On Fri, 2014-09-19 at 22:49 +0200, Jesper Dangaard Brouer wrote:
> This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
> from the qdisc layer, to implement dequeue bulking.
>
> RFC V3: Keeping the ball rolling.
>
> This patchset should now use BQL correctly. I've done lots of testing
> for Head-of-Line blocking issues that can occur due to requeue of a
> SKB bulk list. I've not been able to provoke any HoL blocking
> situation, simply because BQL is doing such a good job, thus I'm
> unable to "overshoot" HW/BQL limits with more than a single packet.
>
> This patch chooses a very conservative approach, as by default only
> allowing dequeue of one extra packet, besides the normal dequeue.
>
> Open questions:
>
> - How do we expose tuning to userspace?
> Patch adds /proc/sys/net/core/qdisc_bulk_dequeue_limit but I don't like it...
> Per device tunable?
>
bql is using /sys, of course ;)
# grep . /sys/class/net/eth1/queues/tx-0/byte_queue_limits/*
/sys/class/net/eth1/queues/tx-0/byte_queue_limits/hold_time:1000
/sys/class/net/eth1/queues/tx-0/byte_queue_limits/inflight:0
/sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit:113314
/sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_max:1879048192
/sys/class/net/eth1/queues/tx-0/byte_queue_limits/limit_min:0
Maybe you could simply reuse byte_queue_limits/limit, I am not sure we
need a specific tunable.
> - Can/should we limit dequeue bulking to devices supporting BQL?
>
Yes please. This will be an incentive to get BQL on drivers.
^ permalink raw reply
* Re: [PATCH net-next] net: bpf: fix compiler warnings in test_bpf
From: Daniel Borkmann @ 2014-09-19 20:56 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S. Miller, Fengguang Wu, netdev
In-Reply-To: <1411160031-3538-1-git-send-email-ast@plumgrid.com>
On 09/19/2014 10:53 PM, Alexei Starovoitov wrote:
> old gcc 4.2 used by avr32 architecture produces warnings:
>
> lib/test_bpf.c:1741: warning: integer constant is too large for 'long' type
> lib/test_bpf.c:1741: warning: integer constant is too large for 'long' type
> lib/test_bpf.c: In function '__run_one':
> lib/test_bpf.c:1897: warning: 'ret' may be used uninitialized in this function
>
> silence these warnings.
>
> Fixes: 02ab695bb37e ("net: filter: add "load 64-bit immediate" eBPF instruction")
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
Acked-by: Daniel Borkmann <dborkman@redhat.com>
^ permalink raw reply
* [PATCH net-next] net: bpf: fix compiler warnings in test_bpf
From: Alexei Starovoitov @ 2014-09-19 20:53 UTC (permalink / raw)
To: David S. Miller; +Cc: Daniel Borkmann, Fengguang Wu, netdev
old gcc 4.2 used by avr32 architecture produces warnings:
lib/test_bpf.c:1741: warning: integer constant is too large for 'long' type
lib/test_bpf.c:1741: warning: integer constant is too large for 'long' type
lib/test_bpf.c: In function '__run_one':
lib/test_bpf.c:1897: warning: 'ret' may be used uninitialized in this function
silence these warnings.
Fixes: 02ab695bb37e ("net: filter: add "load 64-bit immediate" eBPF instruction")
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
lib/test_bpf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 413890815d3e..23e070bcf72d 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1738,7 +1738,7 @@ static struct bpf_test tests[] = {
{
"load 64-bit immediate",
.u.insns_int = {
- BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_LD_IMM64(R1, 0x567800001234LL),
BPF_MOV64_REG(R2, R1),
BPF_MOV64_REG(R3, R2),
BPF_ALU64_IMM(BPF_RSH, R2, 32),
@@ -1894,7 +1894,7 @@ static int __run_one(const struct bpf_prog *fp, const void *data,
int runs, u64 *duration)
{
u64 start, finish;
- int ret, i;
+ int ret = 0, i;
start = ktime_to_us(ktime_get());
--
1.7.9.5
^ permalink raw reply related
* [RFC net-next PATCH V3 2/2] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jesper Dangaard Brouer @ 2014-09-19 20:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
toke
In-Reply-To: <20140919204706.3231.59125.stgit@dragon>
Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
sending/processing an entire skb list.
This patch implements qdisc bulk dequeue, by allowing multiple packets
to be dequeued in dequeue_skb().
The optimization principle for this is two fold, (1) to amortize
locking cost and (2) avoid expensive tailptr update for notifying HW.
(1) Several packets are dequeued while holding the qdisc root_lock,
amortizing locking cost over several packet. The dequeued SKB list is
processed under the TXQ lock in dev_hard_start_xmit(), thus also
amortizing the cost of the TXQ lock.
(2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
API to delay HW tailptr update, which also reduces the cost per
packet.
One restriction of the new API is that every SKB must belong to the
same TXQ. This patch takes the easy way out, by restricting bulk
dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
qdisc only have attached a single TXQ.
Some detail about the flow; dev_hard_start_xmit() will process the skb
list, and transmit packets individually towards the driver (see
xmit_one()). In case the driver stops midway in the list, the
remaining skb list is returned by dev_hard_start_xmit(). In
sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
To avoid overshooting the HW limits, which results in requeuing, the
patch limits the amount of bytes dequeued, based on the drivers BQL
limits.
It also see GSO and segmented GSO packets, as a seperate kind of SKB
bulking, and thus tries to avoid and stop dequeuing when seeing a GSO
packet (both real GSO and segmented GSO skb lists).
Choosing a very conservative initial default value of 1 extra packet
dequeue, if bulking is allowed.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
V2:
- Restruct functions, split out functionality
- Use BQL bytelimit to avoid overshooting driver limits
V3:
- Correct use of BQL
- Some minor adjustments based on feedback.
- Default setting only bulk dequeue 1 extra packet (2 packets).
include/net/sch_generic.h | 2 +
net/core/sysctl_net_core.c | 9 ++++++
net/sched/sch_generic.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 1e89b9a..da9324f 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -14,6 +14,8 @@ struct qdisc_walker;
struct tcf_walker;
struct module;
+extern int qdisc_bulk_dequeue_limit;
+
struct qdisc_rate_table {
struct tc_ratespec rate;
u32 data[256];
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index cf9cd13..5505841 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -361,6 +361,15 @@ static struct ctl_table net_core_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "qdisc_bulk_dequeue_limit",
+ .data = &qdisc_bulk_dequeue_limit,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .extra1 = &zero,
+ .extra2 = &ushort_max,
+ .proc_handler = proc_dointvec_minmax
+ },
{ }
};
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 346ef85..eee2280 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -34,6 +34,9 @@
const struct Qdisc_ops *default_qdisc_ops = &pfifo_fast_ops;
EXPORT_SYMBOL(default_qdisc_ops);
+// FIXME: Still undecided where to put this parameter...
+int qdisc_bulk_dequeue_limit __read_mostly = 1;
+
/* Main transmission queue. */
/* Modifications to data participating in scheduling must be protected with
@@ -56,6 +59,67 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
return 0;
}
+static inline bool qdisc_may_bulk(const struct Qdisc *qdisc)
+{
+ return qdisc->flags & TCQ_F_ONETXQUEUE;
+}
+
+static inline struct sk_buff *qdisc_dequeue_validate(struct Qdisc *qdisc)
+{
+ struct sk_buff *skb = qdisc->dequeue(qdisc);
+
+ if (skb != NULL)
+ skb = validate_xmit_skb(skb, qdisc_dev(qdisc));
+
+ return skb;
+}
+
+static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
+ struct sk_buff *head)
+{
+ struct sk_buff *new, *skb = head;
+ struct netdev_queue *txq = q->dev_queue;
+ int bytelimit = netdev_tx_avail_queue(txq);
+ int limit = qdisc_bulk_dequeue_limit;
+
+ bytelimit -= skb->len; /* incorrect len if skb->next, but exits below */
+
+ // if (bytelimit < psched_mtu(qdisc_dev(q)) //proposed by fw
+ if (bytelimit < 0)
+ return head;
+
+ do {
+ if (skb->next || skb_is_gso(skb)) {
+ /* Stop processing if the skb is already a skb
+ * list (e.g a segmented GSO packet, from
+ * below or func caller) or a real GSO packet
+ */
+ break;
+ }
+ new = q->dequeue(q);
+ if (new) {
+ bytelimit -= new->len; /* covers GSO len */
+ new = validate_xmit_skb(new, qdisc_dev(q));
+ if (!new)
+ break;
+ /* "new" can be a skb list after validate call
+ * above (GSO segmented), but it is okay to
+ * append it to current skb->next, because
+ * next round will exit in-case "new" were a
+ * skb list.
+ */
+ skb->next = new;
+ skb = new;
+ }
+ } while (new && --limit && (bytelimit >= 0));
+ skb = head;
+
+ return skb;
+}
+
+/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
+ * A requeued skb (via q->gso_skb) can also be a SKB list.
+ */
static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
{
struct sk_buff *skb = q->gso_skb;
@@ -71,9 +135,9 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
skb = NULL;
} else {
if (!(q->flags & TCQ_F_ONETXQUEUE) || !netif_xmit_frozen_or_stopped(txq)) {
- skb = q->dequeue(q);
- if (skb)
- skb = validate_xmit_skb(skb, qdisc_dev(q));
+ skb = qdisc_dequeue_validate(q);
+ if (skb && qdisc_may_bulk(q) && qdisc_bulk_dequeue_limit)
+ skb = qdisc_bulk_dequeue_skb(q, skb);
}
}
^ permalink raw reply related
* [RFC net-next PATCH V3 1/2] net: Functions to report space available in device TX queues
From: Jesper Dangaard Brouer @ 2014-09-19 20:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
toke
In-Reply-To: <20140919204706.3231.59125.stgit@dragon>
From: Tom Herbert <therbert@google.com>
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>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
- Fixed spelling in comments
include/linux/netdevice.h | 28 ++++++++++++++++++++++++++--
1 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 28d4378..b12b919 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2551,6 +2551,30 @@ static inline void netdev_completed_queue(struct net_device *dev,
netdev_tx_completed_queue(netdev_get_tx_queue(dev, 0), pkts, bytes);
}
+static inline int netdev_tx_avail_queue(struct netdev_queue *dev_queue)
+{
+#ifdef CONFIG_BQL
+ return dql_avail(&dev_queue->dql);
+#else
+ return DQL_MAX_LIMIT;
+#endif
+}
+
+/**
+ * netdev_avail_queue - report how much space is available for xmit
+ * @dev: network device
+ *
+ * Report the amount of space available in the TX queue in terms of
+ * number of bytes. This returns the number of bytes available per
+ * DQL. This function may be called without taking the txlock on
+ * the device, however in that case the result should be taken as
+ * a (strong) hint.
+ */
+static inline int netdev_avail_queue(struct net_device *dev)
+{
+ return netdev_tx_avail_queue(netdev_get_tx_queue(dev, 0));
+}
+
static inline void netdev_tx_reset_queue(struct netdev_queue *q)
{
#ifdef CONFIG_BQL
@@ -2566,9 +2590,9 @@ static inline void netdev_tx_reset_queue(struct netdev_queue *q)
* Reset the bytes and packet count of a network device and clear the
* software flow control OFF bit for this network device
*/
-static inline void netdev_reset_queue(struct net_device *dev_queue)
+static inline void netdev_reset_queue(struct net_device *dev)
{
- netdev_tx_reset_queue(netdev_get_tx_queue(dev_queue, 0));
+ netdev_tx_reset_queue(netdev_get_tx_queue(dev, 0));
}
/**
^ permalink raw reply related
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