* [PATCH net 1/2] r8152: remove clearing bp
From: Hayes Wang @ 2014-10-01 5:25 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-51-Taiwan-albertk@realtek.com>
The xxx_clear_bp() is used to halt the firmware. It only necessary
for updating the new firmware. Besides, depend on the version of
the current firmware, it may have problem to halt the firmware
directly. Finally, halt the firmware would let the firmware code
useless, and the bugs which are fixed by the firmware would occur.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f68a4e6..18b9e0d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -24,7 +24,7 @@
#include <net/ip6_checksum.h>
/* Version Information */
-#define DRIVER_VERSION "v1.06.0 (2014/03/03)"
+#define DRIVER_VERSION "v1.06.1 (2014/10/01)"
#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
#define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
#define MODULENAME "r8152"
@@ -2201,28 +2201,6 @@ static void rtl_phy_reset(struct r8152 *tp)
}
}
-static void rtl_clear_bp(struct r8152 *tp)
-{
- ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_0, 0);
- ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_2, 0);
- ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_4, 0);
- ocp_write_dword(tp, MCU_TYPE_PLA, PLA_BP_6, 0);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_BP_0, 0);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_BP_2, 0);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_BP_4, 0);
- ocp_write_dword(tp, MCU_TYPE_USB, USB_BP_6, 0);
- mdelay(3);
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_BP_BA, 0);
- ocp_write_word(tp, MCU_TYPE_USB, USB_BP_BA, 0);
-}
-
-static void r8153_clear_bp(struct r8152 *tp)
-{
- ocp_write_byte(tp, MCU_TYPE_PLA, PLA_BP_EN, 0);
- ocp_write_byte(tp, MCU_TYPE_USB, USB_BP_EN, 0);
- rtl_clear_bp(tp);
-}
-
static void r8153_teredo_off(struct r8152 *tp)
{
u32 ocp_data;
@@ -2265,8 +2243,6 @@ static void r8152b_hw_phy_cfg(struct r8152 *tp)
r8152_mdio_write(tp, MII_BMCR, data);
}
- rtl_clear_bp(tp);
-
set_bit(PHY_RESET, &tp->flags);
}
@@ -2417,8 +2393,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8152_mdio_write(tp, MII_BMCR, data);
}
- r8153_clear_bp(tp);
-
if (tp->version == RTL_VER_03) {
data = ocp_reg_read(tp, OCP_EEE_CFG);
data &= ~CTAP_SHORT_EN;
--
1.9.3
^ permalink raw reply related
* [PATCH net 0/2] r8152: patches about firmware
From: Hayes Wang @ 2014-10-01 5:25 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
The patches fix the issues when the firmware exists.
For the multiple OS, the firmware may be loaded by the
driver of the other OS. And the Linux driver has influences
on it.
Hayes Wang (2):
r8152: remove clearing bp
r8152: disable power cut for RTL8153
drivers/net/usb/r8152.c | 30 ++----------------------------
1 file changed, 2 insertions(+), 28 deletions(-)
--
1.9.3
^ permalink raw reply
* RE: How exactly does CHECKSUM_COMPLETE works?
From: Yuval Mintz @ 2014-10-01 5:17 UTC (permalink / raw)
To: Tom Herbert (Partner - google); +Cc: Eric Dumazet, netdev
In-Reply-To: <CA+mtBx8mt5qez5X5tPF_3S+0p2svmLScmxBvXziBYT9XTJ+w0w@mail.gmail.com>
> > Or looking back on the IPv4/TCP scenario, what exactly is the csum now
> > protecting?
> > Assume the HW passes the one's complement sum of the entire data, IP
> > header and onward to driver, which sets it in SKB->csum and marks the
> > SKB as CHECKSUM_COMPLETE.
> > In practice, SKB->csum should now equal to the one's complement of the
> > TCP pseudo header; But other than the length of data in the TCP header
> > and payload that pseudo-header doesn't include any information about
> > the payload itself.
>
> But it does, as you pointed out the checksum calculation was performed over
> the whole payload. The fact that the computed checksum equals the one's
> complement of pseudo header is the indication that the checksum is valid. If
> there were a bit corruption, the computed value would be different (summing in
> pseudo header checksum would result in non-zero value).
Thanks, I've already reached that conclusion myself
[even wrote an E-mail; but it got blocked for some reason].
^ permalink raw reply
* Re: How exactly does CHECKSUM_COMPLETE works?
From: Tom Herbert @ 2014-10-01 5:15 UTC (permalink / raw)
To: Yuval Mintz; +Cc: Eric Dumazet, netdev
In-Reply-To: <B5657A6538887040AD3A81F1008BEC63B415CB@avmb3.qlogic.org>
On Tue, Sep 30, 2014 at 1:32 PM, Yuval Mintz <Yuval.Mintz@qlogic.com> wrote:
>>> IPv4 header is supposed to have a 0 checksum ;)
>>>
>
>>Oops. Sorry, you're right.
>
>>But this is a bit strange.
>
>>I mean, if I go back to the IP-GRE case [with GRE checksum], the IP
>>csum negates the rest of the IP header and the GRE csum negates
>>everything else [GRE header, inner headers and payload].
>>So the mechanism might work as-is, but all the driver has to
>>do in order to claim that an IP-gre packet with checksum option is
>>CHECKSUM_COMPLETE is to write 0xffffffff on the skb's csum field.
>
> Or looking back on the IPv4/TCP scenario, what exactly is the csum
> now protecting?
> Assume the HW passes the one's complement sum of the entire data,
> IP header and onward to driver, which sets it in SKB->csum and marks
> the SKB as CHECKSUM_COMPLETE.
> In practice, SKB->csum should now equal to the one's complement of
> the TCP pseudo header; But other than the length of data in the TCP
> header and payload that pseudo-header doesn't include any
> information about the payload itself.
But it does, as you pointed out the checksum calculation was performed
over the whole payload. The fact that the computed checksum equals the
one's complement of pseudo header is the indication that the checksum
is valid. If there were a bit corruption, the computed value would be
different (summing in pseudo header checksum would result in non-zero
value).
> If we're willing to accept the payload [and the correctness of the
> header while we're at it] based solely on this, what guarantee do we
> have that the TCP checksum is actually correct?
> Seems like the only thing we've guaranteed is that nothing switched in
> IP header since packet passed the HW - but we've already got that
> covered by the IPv4 layer.
>
> [I assume I'm missing something elementary here. Same as before ;-)]
I think your assumption is correct ;-) You might want to take a look
at some of the RFCs that describe checksum verification with pseudo
header.
^ permalink raw reply
* [PATCH net-next] ipv4: mentions skb_gro_postpull_rcsum() in inet_gro_receive()
From: Eric Dumazet @ 2014-10-01 5:12 UTC (permalink / raw)
To: David Miller; +Cc: Yuval.Mintz, therbert, netdev
In-Reply-To: <20140930.161424.679705969149272577.davem@davemloft.net>
From: Eric Dumazet <edumazet@google.com>
Proper CHECKSUM_COMPLETE support needs to adjust skb->csum
when we remove one header. Its done using skb_gro_postpull_rcsum()
In the case of IPv4, we know that the adjustment is not really needed,
because the checksum over IPv4 header is 0. Lets add a comment to
ease code comprehension and avoid copy/paste errors.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/af_inet.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 28e589c5f32d..92db7a69f2b9 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1369,6 +1369,9 @@ static struct sk_buff **inet_gro_receive(struct sk_buff **head,
* immediately following this IP hdr.
*/
+ /* Note : No need to call skb_gro_postpull_rcsum() here,
+ * as we already checked checksum over ipv4 header was 0
+ */
skb_gro_pull(skb, sizeof(*iph));
skb_set_transport_header(skb, skb_gro_offset(skb));
^ permalink raw reply related
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Tom Herbert @ 2014-10-01 4:57 UTC (permalink / raw)
To: Florian Westphal; +Cc: Linux Netdev List, Francois Romieu, Hayes Wang
In-Reply-To: <20140930074259.GC11709@breakpoint.cc>
On Tue, Sep 30, 2014 at 12:42 AM, Florian Westphal <fw@strlen.de> wrote:
> Tom Herbert <therbert@google.com> wrote:
>> Watch inflight and limit in the byte_queue_limits for the queue.
>> inflight must always go back to zero when link goes idle.
>
> Yes, inflight goes to 0 when link is idle.
>
> Output of
> while true; do
> for n in inflight limit; do
> echo -n $n\ ; cat $n;
> done; sleep 1;
> done
>
> during netperf run, 100mbit peer:
>
> inflight 0
> limit 3028
> inflight 6056
> limit 4542
> [ no changes during test ]
> limit 4542
> inflight 3028
> limit 6122
> inflight 0
> limit 6122
> [ changed cable to 1gbit peer, restart netperf ]
> inflight 37850
> limit 36336
> inflight 33308
> limit 31794
> inflight 33308
> limit 31794
> inflight 27252
> limit 25738
> [ no changes during test ]
> inflight 27252
> limit 25738
> inflight 0
> limit 28766
> [ change cable to 100mbit peer, restart netperf ]
> limit 28766
> inflight 27370
> limit 28766
> inflight 4542
> limit 5990
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 6056
> limit 4542
> inflight 0
>
> [ end of test ]
>
> I think thats what its supposed to look like :-)
Looks good!
Acked-by: Tom Herbert <therbert@google.com>
^ permalink raw reply
* [PATCH 1/1 linux-next] net/dccp/ccid.c: add __init to ccid_activate
From: Fabian Frederick @ 2014-10-01 4:52 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
ccid_activate is only called by __init ccid_initialize_builtins in same module.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/ccid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dccp/ccid.c b/net/dccp/ccid.c
index 5975572..8349897 100644
--- a/net/dccp/ccid.c
+++ b/net/dccp/ccid.c
@@ -99,7 +99,7 @@ static void ccid_kmem_cache_destroy(struct kmem_cache *slab)
kmem_cache_destroy(slab);
}
-static int ccid_activate(struct ccid_operations *ccid_ops)
+static int __init ccid_activate(struct ccid_operations *ccid_ops)
{
int err = -ENOBUFS;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 linux-next] net/dccp/proto.c: add __init to dccp_mib_init
From: Fabian Frederick @ 2014-10-01 4:48 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
dccp_mib_init is only called by __init dccp_init in same module.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/proto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index e421edd..6d090c4 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -1082,7 +1082,7 @@ void dccp_shutdown(struct sock *sk, int how)
EXPORT_SYMBOL_GPL(dccp_shutdown);
-static inline int dccp_mib_init(void)
+static inline int __init dccp_mib_init(void)
{
dccp_statistics = alloc_percpu(struct dccp_mib);
if (!dccp_statistics)
--
1.9.3
^ permalink raw reply related
* Re: [net-next PATCH] dql: add a burst attribute
From: Tom Herbert @ 2014-10-01 4:44 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, Florian Westphal, Linux Netdev List,
David S. Miller, Hannes Frederic Sowa, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
Toke Høiland-Jørgensen
In-Reply-To: <20140930234642.3c08b033@redhat.com>
On Tue, Sep 30, 2014 at 2:46 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Tue, 30 Sep 2014 07:55:36 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > On Tue, 2014-09-30 at 16:31 +0200, Florian Westphal wrote:
> > > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > > If you feel not comfortable with "burst", rename it to whatever you
> > > > think is best.
> > > > But please, do not hard code magic 7 in your code.
> > >
> > > I had hoped that this 'magic' value could be removed
> > > completely, only using bql data for bulking decisions.
> >
> > But it is apparently not the case, since you guys decided to had it set
> > to 8, then to 7 later, based on experiments.
>
> The "magic" limit is only a conservative save guard. We do plan to
> remove this completely. That is the reason for not exporting this, as
> we want free-hands to remove this completely.
>
> Guess, I'll remove it completely now, so we can move on.
>
>
> I would like some off/on switch, exported to userspace, for disabling
> this bulking (then we don't need this conservative magic number). How
> would that be done best?
I'd suggest to keep the packet limit as an unsigned. Default value is
infinity (-1UL), 0 turns it off, and other values are available if
someone really wants to tune this (for example, if BQL is disabled--
BQL limit is infinity). Export this as sys variable for the TX queue.
Tom
>
>
> --
> 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 1/1 linux-next] ieee802154: add __init to lowpan_frags_sysctl_register
From: David Miller @ 2014-10-01 4:41 UTC (permalink / raw)
To: fabf; +Cc: alex.aring, netdev, linux-kernel, linux-wpan
In-Reply-To: <1016857334.83918.1412138209231.open-xchange@webmail.nmp.skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 1 Oct 2014 06:36:49 +0200 (CEST)
> I didn't see _ns_ . I'll send the right fix based on wpan-next.
You'll have to send it to me, because I applied your patch already
to net-next.
^ permalink raw reply
* Re: [PATCH 1/1 linux-next] ieee802154: add __init to lowpan_frags_sysctl_register
From: Fabian Frederick @ 2014-10-01 4:36 UTC (permalink / raw)
To: Alexander Aring; +Cc: netdev, linux-kernel, linux-wpan, David S. Miller
In-Reply-To: <20141001002459.GA1007@omega>
> On 01 October 2014 at 02:25 Alexander Aring <alex.aring@gmail.com> wrote:
>
>
> Hi,
>
> On Tue, Sep 30, 2014 at 10:34:08PM +0200, Fabian Frederick wrote:
> > lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init
> > (part of the lowpan module).
> >
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> > This is untested.
> >
> > net/ieee802154/reassembly.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c
> > index 32755cb..30ec608 100644
> > --- a/net/ieee802154/reassembly.c
> > +++ b/net/ieee802154/reassembly.c
> > @@ -485,7 +485,7 @@ static void __net_exit
> > lowpan_frags_ns_sysctl_unregister(struct net *net)
> >
> > static struct ctl_table_header *lowpan_ctl_header;
> >
> > -static int lowpan_frags_sysctl_register(void)
> > +static int __init lowpan_frags_sysctl_register(void)
>
> yes right, but there is more lacks of missing "__init". See below.
>
> > {
> > lowpan_ctl_header = register_net_sysctl(&init_net,
> > "net/ieee802154/6lowpan",
> > @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void)
> > unregister_net_sysctl_table(lowpan_ctl_header);
> > }
> > #else
> > -static inline int lowpan_frags_ns_sysctl_register(struct net *net)
> > +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net)
>
> This is wrong, it's callback from "struct pernet_operations lowpan_frags_ops".
>
> > {
> > return 0;
> > }
>
>
> Your patch adds "__init" now for two different functions and we have
> two different declarations for these functions, depends if CONFIG_SYSCTL is
> enabled.
>
> Now if CONFIG_SYSCTL select we have as lowpan_frags_sysctl_register
> declaration:
>
> static int __init lowpan_frags_sysctl_register(void)
>
> if not:
>
> static inline int lowpan_frags_sysctl_register(void)
>
> Same for lowpan_frags_ns_sysctl_register and vice versa for CONFIG_SYSCTL.
>
> Your changes are for two different functions (Don't know if you realized
> that):
Hi Alexander,
I didn't see _ns_ . I'll send the right fix based on wpan-next.
Thanks,
Fabian
>
> "lowpan_frags_sysctl_register" and "lowpan_frags_ns_sysctl_register",
> which makes no sense. Also lowpan_frags_ns_sysctl_register isn't called
> by module init which is wrong.
>
>
> To make "correct" cleanup for this "__init" should be add to the functions,
> for
> both declarations if CONFIG_SYSCTL is set or not:
>
> - lowpan_net_frag_init
> - lowpan_frags_sysctl_register
>
>
> I see now it's already applied, David please revert this change. Are you
> fine to apply a correct version of this to wpan-next tree, next time?
>
> - Alex
^ permalink raw reply
* Re: [PATCH iproute2] iplink: do not require assigning negative ifindex at link creation
From: Atzm Watanabe @ 2014-10-01 4:30 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Cong Wang, Stephen Hemminger
In-Reply-To: <CAHA+R7NSDU3PeBeJu0jnsZtdmQE=P6zUBH9jram=3gsVZYzcmw@mail.gmail.com>
At Tue, 30 Sep 2014 11:26:04 -0700,
Cong Wang wrote:
>
> On Tue, Sep 30, 2014 at 4:10 AM, Atzm Watanabe <atzm@stratosphere.co.jp> wrote:
> > Since commit 3c682146aeff, iplink requires assigning negative
> > ifindex (-1) to the kernel when creating interface without
> > specifying index.
> >
> > Cc: Cong Wang <xiyou.wangcong@gmail.com>
> > Signed-off-by: Atzm Watanabe <atzm@stratosphere.co.jp>
> > ---
> > ip/iplink.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/ip/iplink.c b/ip/iplink.c
> > index cb9c870..de6b2a9 100644
> > --- a/ip/iplink.c
> > +++ b/ip/iplink.c
> > @@ -689,7 +689,10 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
> > addattr_l(&req.n, sizeof(req), IFLA_LINK, &ifindex, 4);
> > }
> >
> > - req.i.ifi_index = index;
> > + if (index <= 0)
> > + req.i.ifi_index = 0;
> > + else
> > + req.i.ifi_index = index;
>
> How about checking if it is -1?
>
> if (index == -1)
> req.i.ifi_index = 0;
> else
> req.i.ifi_index = index;
Agreed, will fix.
Because iplink_parse() forbids specifying negative index, it will be
actually same. But your suggestion seems more nicer than my patch.
Thanks.
^ permalink raw reply
* Re: [PATCH net v6 4/4] tg3: Fix tx_pending checks for tg3_tso_bug
From: Eric Dumazet @ 2014-10-01 4:24 UTC (permalink / raw)
To: Prashant; +Cc: David Miller, bpoirier, mchan, netdev, linux-kernel
In-Reply-To: <542B71B1.4020103@broadcom.com>
On Tue, 2014-09-30 at 20:14 -0700, Prashant wrote:
> Sorry about the late reply, out of all the HW bug conditions checked in
> tg3_tx_frag_set() the most frequently hit condition is the short 8 byte
> dma bug, where the chip cannot handle TX descriptors whose data buffer
> is 8 bytes or less. Most of the LSO skb's given to the driver has their
> fragments filled upto PAGE_SIZE (expect the last fragment depending on
> skb->len). And if such a LSO skb's last fragment meets the 8 bytes HW
> bug condition the above routine will not help workaround this particular
> case.
Thats pretty easy to work around.
Say rebuilt skb has N frags (N > 1 given your description)
They are numbered 0, ... N-2, N-1
Instead of filling N-2 completely, fill it to PAGE_SIZE-8, so that last
frag has at least 8 bytes in it.
Also take a look at commit 2e4e44107176d552f8bb1bb76053e850e3809841
("net: add alloc_skb_with_frags() helper")
^ permalink raw reply
* Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding doc
From: Richard Cochran @ 2014-10-01 3:59 UTC (permalink / raw)
To: Luwei Zhou
Cc: davem, netdev, shawn.guo, bhutchings, R49496, b38611, b20596,
stephen
In-Reply-To: <20140925144350.GC21453@netboy>
On Thu, Sep 25, 2014 at 04:43:50PM +0200, Richard Cochran wrote:
> On Thu, Sep 25, 2014 at 04:10:21PM +0800, Luwei Zhou wrote:
> > This patch update fec devicetree binding doc that add Optional properties
> > "pps-channel".
>
> Again, use the PTP pin interface. We don't need a random new FEC DT
> property for this.
BTW, if the "channel" only means an internal timer resource, please
find a proper way to claim that resource. Do not add another random DT
property. We have enough of those already.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net v6 4/4] tg3: Fix tx_pending checks for tg3_tso_bug
From: Prashant @ 2014-10-01 3:14 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: bpoirier, mchan, netdev, linux-kernel
In-Reply-To: <20140905.213902.1124686922505260665.davem@davemloft.net>
On 9/5/2014 9:39 PM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Fri, 05 Sep 2014 17:13:06 -0700 (PDT)
>
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 05 Sep 2014 17:03:30 -0700
>>
>>> Instead of this private helper (and pretty limited one btw), we could
>>> add a core function, that would build skbs with order-0 fragments.
>>>
>>> Instead of skb_linearize(), I guess many call sites could instead use
>>> this new helper.
>>>
>>> Because as you said, skb_linearize() of one 64KB GSO packet can ask
>>> order-5 allocations, and this generally does not work reliably.
>>
>> xen-netback could make use of this helper too.
>
> I was curious what it might look like so I cobbled the following
> completely untested patch together :-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index da1378a..eba0ad6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -955,6 +955,67 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
> EXPORT_SYMBOL(skb_copy);
>
> /**
> + * skb_copy_pskb - copy sk_buff into a paged skb
> + * @oskb: buffer to copy
> + * @gfp_mask: allocation priority
> + *
> + * Normalize a paged skb into one that maximally uses order
> + * zero pages in it's fragment array. This is used to canonicalize
> + * spaghetti SKBs that use the page array inefficiently (f.e. only
> + * one byte per page frag).
> + */
> +
> +struct sk_buff *skb_copy_pskb(const struct sk_buff *oskb, gfp_t gfp_mask)
> +{
> + unsigned int data_len = oskb->data_len;
> + int offset, npages, i;
> + struct sk_buff *skb;
> +
> + npages = (data_len + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
> + if (npages > MAX_SKB_FRAGS)
> + return NULL;
> +
> + skb = __alloc_skb(skb_end_offset(oskb), gfp_mask,
> + skb_alloc_rx_flag(oskb), NUMA_NO_NODE);
> + if (!skb)
> + return NULL;
> +
> + skb_reserve(skb, skb_headroom(oskb));
> + skb_put(skb, skb_headlen(oskb));
> + skb_copy_from_linear_data(oskb, skb->data, skb->len);
> +
> + copy_skb_header(skb, oskb);
> +
> + skb->truesize += data_len;
> + offset = skb_headlen(oskb);
> + for (i = 0; i < npages; i++) {
> + struct page *page = alloc_page(gfp_mask);
> + unsigned int chunk;
> + u8 *vaddr;
> +
> + if (!page) {
> + kfree(skb);
> + skb = NULL;
> + break;
> + }
> +
> + chunk = min_t(unsigned int, data_len, PAGE_SIZE);
> + skb_fill_page_desc(skb, i, page, 0, chunk);
> +
> + vaddr = kmap_atomic(page);
> + skb_copy_bits(oskb, offset, vaddr, chunk);
> + kunmap_atomic(vaddr);
> +
> + offset += chunk;
> + data_len -= chunk;
> + skb->data_len += chunk;
> + }
> +
> + return skb;
> +}
> +EXPORT_SYMBOL(skb_copy_pskb);
> +
> +/**
> * __pskb_copy_fclone - create copy of an sk_buff with private head.
> * @skb: buffer to copy
> * @headroom: headroom of new skb
>
Sorry about the late reply, out of all the HW bug conditions checked in
tg3_tx_frag_set() the most frequently hit condition is the short 8 byte
dma bug, where the chip cannot handle TX descriptors whose data buffer
is 8 bytes or less. Most of the LSO skb's given to the driver has their
fragments filled upto PAGE_SIZE (expect the last fragment depending on
skb->len). And if such a LSO skb's last fragment meets the 8 bytes HW
bug condition the above routine will not help workaround this particular
case.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: pktgen: packet bursting via skb->xmit_more
From: Eric Dumazet @ 2014-10-01 1:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Eric Dumazet, Jesper Dangaard Brouer, netdev
In-Reply-To: <1412124801-32096-1-git-send-email-ast@plumgrid.com>
On Tue, 2014-09-30 at 17:53 -0700, Alexei Starovoitov wrote:
> This patch demonstrates the effect of delaying update of HW tailptr.
> (based on earlier patch by Jesper)
>
> burst=1 is the default. It sends one packet with xmit_more=false
> burst=2 sends one packet with xmit_more=true and
> 2nd copy of the same packet with xmit_more=false
> burst=3 sends two copies of the same packet with xmit_more=true and
> 3rd copy with xmit_more=false
>
> Performance with ixgbe (usec 30):
> burst=1 tx:9.2 Mpps
> burst=2 tx:13.5 Mpps
> burst=3 tx:14.5 Mpps full 10G line rate
>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
Very good, thanks Alexei
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the berlin tree
From: Stephen Rothwell @ 2014-10-01 1:22 UTC (permalink / raw)
To: David Miller, netdev, Sebastian Hesselbarth
Cc: linux-next, linux-kernel, Antoine Ténart
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
arch/arm/boot/dts/berlin2q-marvell-dmp.dts between commit 461316fca705
("ARM: berlin: enable the eSATA interface on the BG2Q DMP") from the
berlin tree and commit 75215193b975 ("ARM: dts: berlin: enable the
Ethernet port on the BG2Q DMP") from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 1958cbfe61fa,ea1f99b8eed6..000000000000
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@@ -46,10 -46,6 +46,14 @@@
status = "okay";
};
+&sata0 {
+ status = "okay";
+};
+
+&sata_phy {
+ status = "okay";
+};
++
+ ð0 {
+ status = "okay";
+ };
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Patch net-next] net_sched: avoid calling tcf_unbind_filter() in call_rcu callback
From: John Fastabend @ 2014-10-01 1:18 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, davem, John Fastabend
In-Reply-To: <542B5096.2040106@gmail.com>
On 09/30/2014 05:53 PM, John Fastabend wrote:
> On 09/30/2014 04:07 PM, Cong Wang wrote:
>> This fixes the following crash:
>>
>> [ 63.976822] general protection fault: 0000 [#1] PREEMPT SMP
>> DEBUG_PAGEALLOC
>> [ 63.980094] CPU: 1 PID: 15 Comm: ksoftirqd/1 Not tainted
>> 3.17.0-rc6+ #648
>> [ 63.980094] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>> [ 63.980094] task: ffff880117dea690 ti: ffff880117dfc000 task.ti:
>> ffff880117dfc000
>> [ 63.980094] RIP: 0010:[<ffffffff817e6d07>] [<ffffffff817e6d07>]
>> u32_destroy_key+0x27/0x6d
>> [ 63.980094] RSP: 0018:ffff880117dffcc0 EFLAGS: 00010202
>> [ 63.980094] RAX: ffff880117dea690 RBX: ffff8800d02e0820 RCX:
>> 0000000000000000
>> [ 63.980094] RDX: 0000000000000001 RSI: 0000000000000002 RDI:
>> 6b6b6b6b6b6b6b6b
>> [ 63.980094] RBP: ffff880117dffcd0 R08: 0000000000000000 R09:
>> 0000000000000000
>> [ 63.980094] R10: 00006c0900006ba8 R11: 00006ba100006b9d R12:
>> 0000000000000001
>> [ 63.980094] R13: ffff8800d02e0898 R14: ffffffff817e6d4d R15:
>> ffff880117387a30
>> [ 63.980094] FS: 0000000000000000(0000) GS:ffff88011a800000(0000)
>> knlGS:0000000000000000
>> [ 63.980094] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [ 63.980094] CR2: 00007f07e6732fed CR3: 000000011665b000 CR4:
>> 00000000000006e0
>> [ 63.980094] Stack:
>> [ 63.980094] ffff88011a9cd300 ffffffff82051ac0 ffff880117dffce0
>> ffffffff817e6d68
>> [ 63.980094] ffff880117dffd70 ffffffff810cb4c7 ffffffff810cb3cd
>> ffff880117dfffd8
>> [ 63.980094] ffff880117dea690 ffff880117dea690 ffff880117dfffd8
>> 000000000000000a
>> [ 63.980094] Call Trace:
>> [ 63.980094] [<ffffffff817e6d68>] u32_delete_key_freepf_rcu+0x1b/0x1d
>> [ 63.980094] [<ffffffff810cb4c7>] rcu_process_callbacks+0x3bb/0x691
>> [ 63.980094] [<ffffffff810cb3cd>] ? rcu_process_callbacks+0x2c1/0x691
>> [ 63.980094] [<ffffffff817e6d4d>] ? u32_destroy_key+0x6d/0x6d
>> [ 63.980094] [<ffffffff810780a4>] __do_softirq+0x142/0x323
>> [ 63.980094] [<ffffffff810782a8>] run_ksoftirqd+0x23/0x53
>> [ 63.980094] [<ffffffff81092126>] smpboot_thread_fn+0x203/0x221
>> [ 63.980094] [<ffffffff81091f23>] ? smpboot_unpark_thread+0x33/0x33
>> [ 63.980094] [<ffffffff8108e44d>] kthread+0xc9/0xd1
>> [ 63.980094] [<ffffffff819e00ea>] ? do_wait_for_common+0xf8/0x125
>> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61
>> [ 63.980094] [<ffffffff819e43ec>] ret_from_fork+0x7c/0xb0
>> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61
>>
>> tp could be freed in call_rcu callback too, the order is not guaranteed.
>>
>> Cc: John Fastabend <john.r.fastabend@intel.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> ---
>
> Thanks for catching this. What if we just drop tcf_exts_result
> I can't see how its being used anymore. It appears to just be passed
> around the ./net/sched files for some historic reason that is lost on
> me. Would you mind testing a patch if I sent it out?
>
> Maybe Jamal can shed some light?
>
Sorry I should say its not needed to pass to the actions,
tcf_exts_exec(). It _is_ needed here to get the class setup
correct. And the tcf_exts_exec() stuff is a separate patch.
Thanks again.
Acked-by: John Fastabend <john.r.fastabend@intel.com>
>
>> include/net/pkt_cls.h | 6 +-----
>> net/sched/cls_u32.c | 10 ++++++----
>> 2 files changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>> index 73f9532..ef44ad9 100644
>> --- a/include/net/pkt_cls.h
>> +++ b/include/net/pkt_cls.h
>> @@ -20,11 +20,7 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops
>> *ops);
>> static inline unsigned long
>> __cls_set_class(unsigned long *clp, unsigned long cl)
>> {
>> - unsigned long old_cl;
>> -
>> - old_cl = *clp;
>> - *clp = cl;
>> - return old_cl;
>> + return xchg(clp, cl);
>> }
>>
>> static inline unsigned long
>> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
>> index 4be3ebf..0472909 100644
>> --- a/net/sched/cls_u32.c
>> +++ b/net/sched/cls_u32.c
>> @@ -358,7 +358,6 @@ static int u32_destroy_key(struct tcf_proto *tp,
>> struct tc_u_knode *n,
>> bool free_pf)
>> {
>> - tcf_unbind_filter(tp, &n->res);
>> tcf_exts_destroy(&n->exts);
>> if (n->ht_down)
>> n->ht_down->refcnt--;
>> @@ -416,6 +415,7 @@ static int u32_delete_key(struct tcf_proto *tp,
>> struct tc_u_knode *key)
>> if (pkp == key) {
>> RCU_INIT_POINTER(*kp, key->next);
>>
>> + tcf_unbind_filter(tp, &key->res);
>> call_rcu(&key->rcu, u32_delete_key_freepf_rcu);
>> return 0;
>> }
>> @@ -425,7 +425,7 @@ static int u32_delete_key(struct tcf_proto *tp,
>> struct tc_u_knode *key)
>> return 0;
>> }
>>
>> -static void u32_clear_hnode(struct tc_u_hnode *ht)
>> +static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
>> {
>> struct tc_u_knode *n;
>> unsigned int h;
>> @@ -434,6 +434,7 @@ static void u32_clear_hnode(struct tc_u_hnode *ht)
>> while ((n = rtnl_dereference(ht->ht[h])) != NULL) {
>> RCU_INIT_POINTER(ht->ht[h],
>> rtnl_dereference(n->next));
>> + tcf_unbind_filter(tp, &n->res);
>> call_rcu(&n->rcu, u32_delete_key_freepf_rcu);
>> }
>> }
>> @@ -447,7 +448,7 @@ static int u32_destroy_hnode(struct tcf_proto *tp,
>> struct tc_u_hnode *ht)
>>
>> WARN_ON(ht->refcnt);
>>
>> - u32_clear_hnode(ht);
>> + u32_clear_hnode(tp, ht);
>>
>> hn = &tp_c->hlist;
>> for (phn = rtnl_dereference(*hn);
>> @@ -482,7 +483,7 @@ static void u32_destroy(struct tcf_proto *tp)
>> ht;
>> ht = rtnl_dereference(ht->next)) {
>> ht->refcnt--;
>> - u32_clear_hnode(ht);
>> + u32_clear_hnode(tp, ht);
>> }
>>
>> while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) {
>> @@ -731,6 +732,7 @@ static int u32_change(struct net *net, struct
>> sk_buff *in_skb,
>> }
>>
>> u32_replace_knode(tp, tp_c, new);
>> + tcf_unbind_filter(tp, &n->res);
>> call_rcu(&n->rcu, u32_delete_key_rcu);
>> return 0;
>> }
>>
>
>
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [Patch net-next] net_sched: avoid calling tcf_unbind_filter() in call_rcu callback
From: John Fastabend @ 2014-10-01 0:53 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, davem, John Fastabend
In-Reply-To: <1412118444-29179-2-git-send-email-xiyou.wangcong@gmail.com>
On 09/30/2014 04:07 PM, Cong Wang wrote:
> This fixes the following crash:
>
> [ 63.976822] general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ 63.980094] CPU: 1 PID: 15 Comm: ksoftirqd/1 Not tainted 3.17.0-rc6+ #648
> [ 63.980094] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 63.980094] task: ffff880117dea690 ti: ffff880117dfc000 task.ti: ffff880117dfc000
> [ 63.980094] RIP: 0010:[<ffffffff817e6d07>] [<ffffffff817e6d07>] u32_destroy_key+0x27/0x6d
> [ 63.980094] RSP: 0018:ffff880117dffcc0 EFLAGS: 00010202
> [ 63.980094] RAX: ffff880117dea690 RBX: ffff8800d02e0820 RCX: 0000000000000000
> [ 63.980094] RDX: 0000000000000001 RSI: 0000000000000002 RDI: 6b6b6b6b6b6b6b6b
> [ 63.980094] RBP: ffff880117dffcd0 R08: 0000000000000000 R09: 0000000000000000
> [ 63.980094] R10: 00006c0900006ba8 R11: 00006ba100006b9d R12: 0000000000000001
> [ 63.980094] R13: ffff8800d02e0898 R14: ffffffff817e6d4d R15: ffff880117387a30
> [ 63.980094] FS: 0000000000000000(0000) GS:ffff88011a800000(0000) knlGS:0000000000000000
> [ 63.980094] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 63.980094] CR2: 00007f07e6732fed CR3: 000000011665b000 CR4: 00000000000006e0
> [ 63.980094] Stack:
> [ 63.980094] ffff88011a9cd300 ffffffff82051ac0 ffff880117dffce0 ffffffff817e6d68
> [ 63.980094] ffff880117dffd70 ffffffff810cb4c7 ffffffff810cb3cd ffff880117dfffd8
> [ 63.980094] ffff880117dea690 ffff880117dea690 ffff880117dfffd8 000000000000000a
> [ 63.980094] Call Trace:
> [ 63.980094] [<ffffffff817e6d68>] u32_delete_key_freepf_rcu+0x1b/0x1d
> [ 63.980094] [<ffffffff810cb4c7>] rcu_process_callbacks+0x3bb/0x691
> [ 63.980094] [<ffffffff810cb3cd>] ? rcu_process_callbacks+0x2c1/0x691
> [ 63.980094] [<ffffffff817e6d4d>] ? u32_destroy_key+0x6d/0x6d
> [ 63.980094] [<ffffffff810780a4>] __do_softirq+0x142/0x323
> [ 63.980094] [<ffffffff810782a8>] run_ksoftirqd+0x23/0x53
> [ 63.980094] [<ffffffff81092126>] smpboot_thread_fn+0x203/0x221
> [ 63.980094] [<ffffffff81091f23>] ? smpboot_unpark_thread+0x33/0x33
> [ 63.980094] [<ffffffff8108e44d>] kthread+0xc9/0xd1
> [ 63.980094] [<ffffffff819e00ea>] ? do_wait_for_common+0xf8/0x125
> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61
> [ 63.980094] [<ffffffff819e43ec>] ret_from_fork+0x7c/0xb0
> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61
>
> tp could be freed in call_rcu callback too, the order is not guaranteed.
>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
Thanks for catching this. What if we just drop tcf_exts_result
I can't see how its being used anymore. It appears to just be passed
around the ./net/sched files for some historic reason that is lost on
me. Would you mind testing a patch if I sent it out?
Maybe Jamal can shed some light?
> include/net/pkt_cls.h | 6 +-----
> net/sched/cls_u32.c | 10 ++++++----
> 2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 73f9532..ef44ad9 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -20,11 +20,7 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
> static inline unsigned long
> __cls_set_class(unsigned long *clp, unsigned long cl)
> {
> - unsigned long old_cl;
> -
> - old_cl = *clp;
> - *clp = cl;
> - return old_cl;
> + return xchg(clp, cl);
> }
>
> static inline unsigned long
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index 4be3ebf..0472909 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -358,7 +358,6 @@ static int u32_destroy_key(struct tcf_proto *tp,
> struct tc_u_knode *n,
> bool free_pf)
> {
> - tcf_unbind_filter(tp, &n->res);
> tcf_exts_destroy(&n->exts);
> if (n->ht_down)
> n->ht_down->refcnt--;
> @@ -416,6 +415,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
> if (pkp == key) {
> RCU_INIT_POINTER(*kp, key->next);
>
> + tcf_unbind_filter(tp, &key->res);
> call_rcu(&key->rcu, u32_delete_key_freepf_rcu);
> return 0;
> }
> @@ -425,7 +425,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
> return 0;
> }
>
> -static void u32_clear_hnode(struct tc_u_hnode *ht)
> +static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
> {
> struct tc_u_knode *n;
> unsigned int h;
> @@ -434,6 +434,7 @@ static void u32_clear_hnode(struct tc_u_hnode *ht)
> while ((n = rtnl_dereference(ht->ht[h])) != NULL) {
> RCU_INIT_POINTER(ht->ht[h],
> rtnl_dereference(n->next));
> + tcf_unbind_filter(tp, &n->res);
> call_rcu(&n->rcu, u32_delete_key_freepf_rcu);
> }
> }
> @@ -447,7 +448,7 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
>
> WARN_ON(ht->refcnt);
>
> - u32_clear_hnode(ht);
> + u32_clear_hnode(tp, ht);
>
> hn = &tp_c->hlist;
> for (phn = rtnl_dereference(*hn);
> @@ -482,7 +483,7 @@ static void u32_destroy(struct tcf_proto *tp)
> ht;
> ht = rtnl_dereference(ht->next)) {
> ht->refcnt--;
> - u32_clear_hnode(ht);
> + u32_clear_hnode(tp, ht);
> }
>
> while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) {
> @@ -731,6 +732,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
> }
>
> u32_replace_knode(tp, tp_c, new);
> + tcf_unbind_filter(tp, &n->res);
> call_rcu(&n->rcu, u32_delete_key_rcu);
> return 0;
> }
>
--
John Fastabend Intel Corporation
^ permalink raw reply
* [PATCH v2 net-next] net: pktgen: packet bursting via skb->xmit_more
From: Alexei Starovoitov @ 2014-10-01 0:53 UTC (permalink / raw)
To: David S. Miller; +Cc: Eric Dumazet, Jesper Dangaard Brouer, netdev
This patch demonstrates the effect of delaying update of HW tailptr.
(based on earlier patch by Jesper)
burst=1 is the default. It sends one packet with xmit_more=false
burst=2 sends one packet with xmit_more=true and
2nd copy of the same packet with xmit_more=false
burst=3 sends two copies of the same packet with xmit_more=true and
3rd copy with xmit_more=false
Performance with ixgbe (usec 30):
burst=1 tx:9.2 Mpps
burst=2 tx:13.5 Mpps
burst=3 tx:14.5 Mpps full 10G line rate
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v1 -> v2:
- simplified burst counting logic (suggested by Eric)
- re-tested on ixgbe, mlx4, bnx2x
RFC -> v1:
- added netif_xmit_frozen_or_drv_stopped() check, since bnx2x
stops the queue and returns TX_OK (suggested by Eric)
- read pkt_dev->burst once to avoid races (suggested by Eric)
- changed 'int burst' to 'unsigned int burst'
- updated doc
- tested on ixgbe, mlx4, bnx2x
Comparing to Jesper patch this one amortizes the cost
of spin_lock and atomic_inc by doing HARD_TX_LOCK and
atomic_add(N) once across N packets.
Documentation/networking/pktgen.txt | 3 +++
net/core/pktgen.c | 26 ++++++++++++++++++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt
index 0dffc6e37902..6915c6b27869 100644
--- a/Documentation/networking/pktgen.txt
+++ b/Documentation/networking/pktgen.txt
@@ -99,6 +99,9 @@ Examples:
pgset "clone_skb 1" sets the number of copies of the same packet
pgset "clone_skb 0" use single SKB for all transmits
+ pgset "burst 8" uses xmit_more API to queue 8 copies of the same
+ packet and update HW tx queue tail pointer once.
+ "burst 1" is the default
pgset "pkt_size 9014" sets packet size to 9014
pgset "frags 5" packet will consist of 5 fragments
pgset "count 200000" sets number of packets to send, set to zero
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 5c728aaf8d6c..443256bdcddc 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -387,6 +387,7 @@ struct pktgen_dev {
u16 queue_map_min;
u16 queue_map_max;
__u32 skb_priority; /* skb priority field */
+ unsigned int burst; /* number of duplicated packets to burst */
int node; /* Memory node */
#ifdef CONFIG_XFRM
@@ -613,6 +614,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
if (pkt_dev->traffic_class)
seq_printf(seq, " traffic_class: 0x%02x\n", pkt_dev->traffic_class);
+ if (pkt_dev->burst > 1)
+ seq_printf(seq, " burst: %d\n", pkt_dev->burst);
+
if (pkt_dev->node >= 0)
seq_printf(seq, " node: %d\n", pkt_dev->node);
@@ -1124,6 +1128,16 @@ static ssize_t pktgen_if_write(struct file *file,
pkt_dev->dst_mac_count);
return count;
}
+ if (!strcmp(name, "burst")) {
+ len = num_arg(&user_buffer[i], 10, &value);
+ if (len < 0)
+ return len;
+
+ i += len;
+ pkt_dev->burst = value < 1 ? 1 : value;
+ sprintf(pg_result, "OK: burst=%d", pkt_dev->burst);
+ return count;
+ }
if (!strcmp(name, "node")) {
len = num_arg(&user_buffer[i], 10, &value);
if (len < 0)
@@ -3297,6 +3311,7 @@ static void pktgen_wait_for_skb(struct pktgen_dev *pkt_dev)
static void pktgen_xmit(struct pktgen_dev *pkt_dev)
{
+ unsigned int burst = ACCESS_ONCE(pkt_dev->burst);
struct net_device *odev = pkt_dev->odev;
struct netdev_queue *txq;
int ret;
@@ -3347,8 +3362,10 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
pkt_dev->last_ok = 0;
goto unlock;
}
- atomic_inc(&(pkt_dev->skb->users));
- ret = netdev_start_xmit(pkt_dev->skb, odev, txq, false);
+ atomic_add(burst, &pkt_dev->skb->users);
+
+xmit_more:
+ ret = netdev_start_xmit(pkt_dev->skb, odev, txq, --burst > 0);
switch (ret) {
case NETDEV_TX_OK:
@@ -3356,6 +3373,8 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
pkt_dev->sofar++;
pkt_dev->seq_num++;
pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
+ if (burst > 0 && !netif_xmit_frozen_or_drv_stopped(txq))
+ goto xmit_more;
break;
case NET_XMIT_DROP:
case NET_XMIT_CN:
@@ -3374,6 +3393,8 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
atomic_dec(&(pkt_dev->skb->users));
pkt_dev->last_ok = 0;
}
+ if (unlikely(burst))
+ atomic_sub(burst, &pkt_dev->skb->users);
unlock:
HARD_TX_UNLOCK(odev, txq);
@@ -3572,6 +3593,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
pkt_dev->svlan_p = 0;
pkt_dev->svlan_cfi = 0;
pkt_dev->svlan_id = 0xffff;
+ pkt_dev->burst = 1;
pkt_dev->node = -1;
err = pktgen_setup_dev(t->net, pkt_dev, ifname);
--
1.7.9.5
^ permalink raw reply related
* Re: Philosophical question: Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Rick Jones @ 2014-10-01 0:31 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1412123358.16704.22.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/30/2014 05:29 PM, Eric Dumazet wrote:
> Yes, please submit your patch formally, maybe using a more complete
> form ? ;)
Will do.
rick
^ permalink raw reply
* Re: Philosophical question: Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Eric Dumazet @ 2014-10-01 0:29 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <542B4950.8050301@hp.com>
On Tue, 2014-09-30 at 17:22 -0700, Rick Jones wrote:
> On 09/30/2014 04:23 PM, Eric Dumazet wrote:
> > On Tue, 2014-09-30 at 16:09 -0700, Rick Jones wrote:
> >> I've been looking at some additional perf <mutter> -e skb_kfree_skb
> >> results, this time with a laptop connected to a corporate network with a
> >> large number of Windows systems sending out what they are wont to
> >> send... The laptop is just sitting there no active netperfs or anything :)
> >>
> >> I see profile hits for __udp4_lib_mcast_deliver() which has a
> >> kfree_skb() call which will happen if either there were no sockets
> >> found, or if an integral multiple of ARRAY_SIZE(stack) sockets are
> >> found. I'm assuming the latter is exceedingly rare.
> >>
> >> Anywho, the philosophical question - is such a situation a drop
> >> (indicating the existing kfree_skb()), or is it an ignore (indicating a
> >> consume_skb())? Should there be a statistic incremented for either of
> >> those?
> >
> > I guess we lack a UDP_MIB_NOPORTS increase here.
>
> I was going back and forth on that - since it is a multicast it may not
> have really been directed at us in which case it would be an ignore (and
> perhaps a new "ignored" stat?). But on the assumption that it should
> indeed remain a drop, and so a kfree_skb(), something along the lines of:
>
>
> ? The idea being that in the unlikely event there were indeed enough
> matches to trigger the flush_stack in the for_each and only enough for
> that it will be a consume_skb() and no statistic rather than a
> kfree_skb() and a statistic increment.
Yes, please submit your patch formally, maybe using a more complete
form ? ;)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db5471bb5..be7db86046af 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1656,6 +1656,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
int dif = skb->dev->ifindex;
unsigned int count = 0, offset = offsetof(typeof(*sk), sk_nulls_node);
unsigned int hash2 = 0, hash2_any = 0, use_hash2 = (hslot->count > 10);
+ bool delivered = false;
if (use_hash2) {
hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY), hnum) &
@@ -1674,6 +1675,7 @@ start_lookup:
dif, hnum)) {
if (unlikely(count == ARRAY_SIZE(stack))) {
flush_stack(stack, count, skb, ~0);
+ delivered = true;
count = 0;
}
stack[count++] = sk;
@@ -1694,8 +1696,11 @@ start_lookup:
*/
if (count) {
flush_stack(stack, count, skb, count - 1);
- } else {
+ } else if (!delivered) {
+ UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, proto == IPPROTO_UDPLITE);
kfree_skb(skb);
+ } else {
+ consume_skb(skb);
}
return 0;
}
^ permalink raw reply related
* Re: [PATCH 1/1 linux-next] ieee802154: add __init to lowpan_frags_sysctl_register
From: Alexander Aring @ 2014-10-01 0:25 UTC (permalink / raw)
To: Fabian Frederick; +Cc: linux-kernel, David S. Miller, linux-wpan, netdev
In-Reply-To: <1412109249-7432-1-git-send-email-fabf@skynet.be>
Hi,
On Tue, Sep 30, 2014 at 10:34:08PM +0200, Fabian Frederick wrote:
> lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init
> (part of the lowpan module).
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> This is untested.
>
> net/ieee802154/reassembly.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c
> index 32755cb..30ec608 100644
> --- a/net/ieee802154/reassembly.c
> +++ b/net/ieee802154/reassembly.c
> @@ -485,7 +485,7 @@ static void __net_exit lowpan_frags_ns_sysctl_unregister(struct net *net)
>
> static struct ctl_table_header *lowpan_ctl_header;
>
> -static int lowpan_frags_sysctl_register(void)
> +static int __init lowpan_frags_sysctl_register(void)
yes right, but there is more lacks of missing "__init". See below.
> {
> lowpan_ctl_header = register_net_sysctl(&init_net,
> "net/ieee802154/6lowpan",
> @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void)
> unregister_net_sysctl_table(lowpan_ctl_header);
> }
> #else
> -static inline int lowpan_frags_ns_sysctl_register(struct net *net)
> +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net)
This is wrong, it's callback from "struct pernet_operations lowpan_frags_ops".
> {
> return 0;
> }
Your patch adds "__init" now for two different functions and we have
two different declarations for these functions, depends if CONFIG_SYSCTL is
enabled.
Now if CONFIG_SYSCTL select we have as lowpan_frags_sysctl_register declaration:
static int __init lowpan_frags_sysctl_register(void)
if not:
static inline int lowpan_frags_sysctl_register(void)
Same for lowpan_frags_ns_sysctl_register and vice versa for CONFIG_SYSCTL.
Your changes are for two different functions (Don't know if you realized that):
"lowpan_frags_sysctl_register" and "lowpan_frags_ns_sysctl_register",
which makes no sense. Also lowpan_frags_ns_sysctl_register isn't called
by module init which is wrong.
To make "correct" cleanup for this "__init" should be add to the functions, for
both declarations if CONFIG_SYSCTL is set or not:
- lowpan_net_frag_init
- lowpan_frags_sysctl_register
I see now it's already applied, David please revert this change. Are you
fine to apply a correct version of this to wpan-next tree, next time?
- Alex
^ permalink raw reply
* Re: Philosophical question: Is a UDP multicast datagram for which there is no socket match a drop or an ignore?
From: Rick Jones @ 2014-10-01 0:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1412119398.16704.11.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/30/2014 04:23 PM, Eric Dumazet wrote:
> On Tue, 2014-09-30 at 16:09 -0700, Rick Jones wrote:
>> I've been looking at some additional perf <mutter> -e skb_kfree_skb
>> results, this time with a laptop connected to a corporate network with a
>> large number of Windows systems sending out what they are wont to
>> send... The laptop is just sitting there no active netperfs or anything :)
>>
>> I see profile hits for __udp4_lib_mcast_deliver() which has a
>> kfree_skb() call which will happen if either there were no sockets
>> found, or if an integral multiple of ARRAY_SIZE(stack) sockets are
>> found. I'm assuming the latter is exceedingly rare.
>>
>> Anywho, the philosophical question - is such a situation a drop
>> (indicating the existing kfree_skb()), or is it an ignore (indicating a
>> consume_skb())? Should there be a statistic incremented for either of
>> those?
>
> I guess we lack a UDP_MIB_NOPORTS increase here.
I was going back and forth on that - since it is a multicast it may not
have really been directed at us in which case it would be an ignore (and
perhaps a new "ignored" stat?). But on the assumption that it should
indeed remain a drop, and so a kfree_skb(), something along the lines of:
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db54..376e3d3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1656,6 +1656,7 @@ static int __udp4_lib_mcast_deliver(struct net
*net, struc
int dif = skb->dev->ifindex;
unsigned int count = 0, offset = offsetof(typeof(*sk),
sk_nulls_node);
unsigned int hash2 = 0, hash2_any = 0, use_hash2 =
(hslot->count > 10);
+ unsigned int inner_flushed = 0;
if (use_hash2) {
hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY),
hnum) &
@@ -1694,8 +1695,12 @@ start_lookup:
*/
if (count) {
flush_stack(stack, count, skb, count - 1);
- } else {
+ } else if (!inner_flushed) {
+ UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, 0);
kfree_skb(skb);
+ } else {
+ /* there were matches flushed in the for_each */
+ consume_skb(skb);
}
return 0;
}
? The idea being that in the unlikely event there were indeed enough
matches to trigger the flush_stack in the for_each and only enough for
that it will be a consume_skb() and no statistic rather than a
kfree_skb() and a statistic increment.
(likely munged by my mailer)
rick
^ permalink raw reply related
* Re: [PATCH net-next] net: pktgen: packet bursting via skb->xmit_more
From: Alexei Starovoitov @ 2014-10-01 0:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Eric Dumazet, Jesper Dangaard Brouer,
Network Development
In-Reply-To: <1412122303.16704.15.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Sep 30, 2014 at 5:11 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> It seems you dont really need @burst_cnt or @more in this function.
excellent suggestion! That is indeed much easier to read. will respin.
^ 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