* Re: [RFC] tcp: randomize TCP source ports
From: Daniel Borkmann @ 2013-11-09 18:16 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <20131109044724.GB1963@order.stressinduktion.org>
On 11/09/2013 05:47 AM, Hannes Frederic Sowa wrote:
> On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
>> On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
>>
>>> What do you think about using a timer to keep the reseed out of fast-path
>>> and switch to the non-arch get_random_bytes instead?
>>
>> Well, the initial seed value is get_random_bytes(). I felt that using a
>> xor with the _arch() version would be safe enough.
>>
>> For the timer, I do not think its worth the pain : Do you want a per cpu
>> timer, or a global one ?
>
> This untested diff came to my mind (it is based on the random tree). I
> actually consider to propose something like this for 3.13. UDP port
> randomization is really critical.
>
> In 3.14 timeframe I suggest abandon net_random and use prandom_u32
> directly so code gets easier to audit.
>
> Would it hurt to use "proper" get_random_byte calls for port randomization?
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index cdf4cfb..e9d0136 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -657,9 +657,11 @@ retry:
> r->entropy_total += nbits;
> if (!r->initialized && nbits > 0) {
> if (r->entropy_total > 128) {
> - if (r == &nonblocking_pool)
> + if (r == &nonblocking_pool) {
> pr_notice("random: %s pool is initialized\n",
> r->name);
> + prandom_reseed();
> + }
> r->initialized = 1;
> r->entropy_total = 0;
> }
> diff --git a/include/linux/random.h b/include/linux/random.h
> index 6312dd9..4f878c0 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
> u32 prandom_u32(void);
> void prandom_bytes(void *buf, int nbytes);
> void prandom_seed(u32 seed);
> +void prandom_reseed(void);
>
> u32 prandom_u32_state(struct rnd_state *);
> void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
> diff --git a/lib/random32.c b/lib/random32.c
> index 52280d5..1ee611f 100644
> --- a/lib/random32.c
> +++ b/lib/random32.c
> @@ -174,11 +174,31 @@ static int __init prandom_init(void)
> }
> core_initcall(prandom_init);
>
> +static void __prandom_timer(unsigned long dontcare);
> +static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
> +
> +static void __prandom_timer(unsigned long dontcare)
> +{
> + u32 entropy;
> + get_random_bytes(&entropy, sizeof(entropy));
> + prandom_seed(entropy);
> + seed_timer.expires = jiffies + 60 * HZ;
> + add_timer(&seed_timer);
> +}
> +
> +static int prandom_start_seed_timer(void)
^^^^^^ __init
> +{
prandom_reseed();
What are the objectives against initializing prandom here in
the late initcall [instead of doing so in drivers/char/random.c]
as it was the case before?
Probably for security reasons, I think you actually don't want
anyone (incl. external 3rd party modules) to call prandom_reseed()
again after this has been done once initially. So I think it
would be better to make this function not visible to anyone
outside of random32.c.
> + seed_timer.expires = jiffies + 60 * HZ;
> + add_timer(&seed_timer);
> + return 0;
> +}
> +late_initcall(prandom_start_seed_timer);
> +
> /*
> * Generate better values after random number generator
> * is fully initialized.
> */
> -static int __init prandom_reseed(void)
> +void prandom_reseed(void)
> {
> int i;
>
> @@ -196,4 +216,3 @@ static int __init prandom_reseed(void)
> }
> return 0;
> }
> -late_initcall(prandom_reseed);
>
> Greetings,
>
> Hannes
^ permalink raw reply
* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: David Miller @ 2013-11-09 19:27 UTC (permalink / raw)
To: jhs; +Cc: tgraf, jbenc, netdev, pablo
In-Reply-To: <527E3C17.1080508@mojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sat, 09 Nov 2013 08:43:51 -0500
> for errors, we need to give the user something back. This has been the
> behavior for 80 years now. Giving them a HUGE message
> back is rediculuos(tm). Ive had enough of SCTP doing that.
> We need to cap it - sort of what ICMP does.
> ICMP caps at 64B; something like 128B is reasonable.
It is correct that we really can't change existing behavior.
I want to do something smarter in the new cases where we can.
nftables is the first thing that works with such enormous
messages, so let's create a facility such that nftables
netlink users don't need to get the entire quote message
back.
That's why I suggested a per-subsystem flag, that entities like
nftables can set when it registers, that says "don't quote the message
in the ACK."
Or, alternatively, let's have the application set this flag,
via a socket option or similar.
Both approaches work for me, and the latter probably gains us
the most over time as we can make sure that eventually all the
major netlink apps start setting the flag.
^ permalink raw reply
* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: Pablo Neira Ayuso @ 2013-11-09 19:49 UTC (permalink / raw)
To: David Miller; +Cc: jhs, tgraf, jbenc, netdev
In-Reply-To: <20131109.142706.307323939750387593.davem@davemloft.net>
On Sat, Nov 09, 2013 at 02:27:06PM -0500, David Miller wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> Date: Sat, 09 Nov 2013 08:43:51 -0500
>
> > for errors, we need to give the user something back. This has been the
> > behavior for 80 years now. Giving them a HUGE message
> > back is rediculuos(tm). Ive had enough of SCTP doing that.
> > We need to cap it - sort of what ICMP does.
> > ICMP caps at 64B; something like 128B is reasonable.
>
> It is correct that we really can't change existing behavior.
>
> I want to do something smarter in the new cases where we can.
>
> nftables is the first thing that works with such enormous
> messages, so let's create a facility such that nftables
> netlink users don't need to get the entire quote message
> back.
>
> That's why I suggested a per-subsystem flag, that entities like
> nftables can set when it registers, that says "don't quote the message
> in the ACK."
>
> Or, alternatively, let's have the application set this flag,
> via a socket option or similar.
>
> Both approaches work for me, and the latter probably gains us
> the most over time as we can make sure that eventually all the
> major netlink apps start setting the flag.
In the nftables case, we send a large packet containing small netlink
messages, so it's unlikely that we'll hit the problem that Jiri
reported since the ack is reported back per small message in the
packet.
But we still have to fix this for other netlink subsystems following
either approach, David's flag or Jamal's netlink error with origin
netlink message cap.
^ permalink raw reply
* Re: [RFC PATCH 0/2] Improve tracing at the driver/core boundary
From: David Miller @ 2013-11-09 20:20 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1381761552.1626.8.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 14 Oct 2013 15:39:12 +0100
> These patches add static tracpeoints at the driver/core boundary which
> record various skb fields likely to be useful for datapath debugging.
> On the TX side the boundary is where the core calls ndo_start_xmit, and
> on the RX side it is where any of the various exported receive functions
> is called.
>
> The set of skb fields is mostly based on what I thought would be
> interesting for sfc, and may need to be augmented to be more general.
FWIW I think these changes are fine and a good idea, thanks for following
up on this Ben.
^ permalink raw reply
* Re: [RFC] tcp: randomize TCP source ports
From: Hannes Frederic Sowa @ 2013-11-09 20:54 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <527E7BEA.1070904@redhat.com>
On Sat, Nov 09, 2013 at 07:16:10PM +0100, Daniel Borkmann wrote:
> On 11/09/2013 05:47 AM, Hannes Frederic Sowa wrote:
> >On Fri, Nov 08, 2013 at 07:11:18AM -0800, Eric Dumazet wrote:
> >>On Fri, 2013-11-08 at 15:28 +0100, Hannes Frederic Sowa wrote:
> >>
> >>>What do you think about using a timer to keep the reseed out of fast-path
> >>>and switch to the non-arch get_random_bytes instead?
> >>
> >>Well, the initial seed value is get_random_bytes(). I felt that using a
> >>xor with the _arch() version would be safe enough.
> >>
> >>For the timer, I do not think its worth the pain : Do you want a per cpu
> >>timer, or a global one ?
> >
> >This untested diff came to my mind (it is based on the random tree). I
> >actually consider to propose something like this for 3.13. UDP port
> >randomization is really critical.
> >
> >In 3.14 timeframe I suggest abandon net_random and use prandom_u32
> >directly so code gets easier to audit.
> >
> >Would it hurt to use "proper" get_random_byte calls for port randomization?
> >
> >diff --git a/drivers/char/random.c b/drivers/char/random.c
> >index cdf4cfb..e9d0136 100644
> >--- a/drivers/char/random.c
> >+++ b/drivers/char/random.c
> >@@ -657,9 +657,11 @@ retry:
> > r->entropy_total += nbits;
> > if (!r->initialized && nbits > 0) {
> > if (r->entropy_total > 128) {
> >- if (r == &nonblocking_pool)
> >+ if (r == &nonblocking_pool) {
> > pr_notice("random: %s pool is initialized\n",
> > r->name);
> >+ prandom_reseed();
> >+ }
> > r->initialized = 1;
> > r->entropy_total = 0;
I rearranged the code so get_random_bytes does not emit a warning when called
from prandom_reseed().
> > }
> >diff --git a/include/linux/random.h b/include/linux/random.h
> >index 6312dd9..4f878c0 100644
> >--- a/include/linux/random.h
> >+++ b/include/linux/random.h
> >@@ -29,6 +29,7 @@ unsigned long randomize_range(unsigned long start,
> >unsigned long end, unsigned l
> > u32 prandom_u32(void);
> > void prandom_bytes(void *buf, int nbytes);
> > void prandom_seed(u32 seed);
> >+void prandom_reseed(void);
> >
> > u32 prandom_u32_state(struct rnd_state *);
> > void prandom_bytes_state(struct rnd_state *state, void *buf, int nbytes);
> >diff --git a/lib/random32.c b/lib/random32.c
> >index 52280d5..1ee611f 100644
> >--- a/lib/random32.c
> >+++ b/lib/random32.c
> >@@ -174,11 +174,31 @@ static int __init prandom_init(void)
> > }
> > core_initcall(prandom_init);
> >
> >+static void __prandom_timer(unsigned long dontcare);
> >+static DEFINE_TIMER(seed_timer, __prandom_timer, 0, 0);
> >+
> >+static void __prandom_timer(unsigned long dontcare)
> >+{
> >+ u32 entropy;
> >+ get_random_bytes(&entropy, sizeof(entropy));
> >+ prandom_seed(entropy);
> >+ seed_timer.expires = jiffies + 60 * HZ;
> >+ add_timer(&seed_timer);
> >+}
> >+
> >+static int prandom_start_seed_timer(void)
>
> ^^^^^^ __init
Also fixed in my commit. Thanks!
> >+{
> prandom_reseed();
>
> What are the objectives against initializing prandom here in
> the late initcall [instead of doing so in drivers/char/random.c]
> as it was the case before?
IMHO even the late initcall is way too early to seed the prng properly.
Later reseeds never touch s2 and s3 of rnd_state again, so I want to
make sure we have a proper initialized entropy pool when we do the
initial prandom_reseed().
> Probably for security reasons, I think you actually don't want
> anyone (incl. external 3rd party modules) to call prandom_reseed()
> again after this has been done once initially. So I think it
> would be better to make this function not visible to anyone
> outside of random32.c.
It would be nice but a later call to prandom_reseed would not hurt that
much as a too early one. So I guess the tradeoff is worth it.
Maybe we can add runtime protection so it only will get called once.
I am also thinking about leaving the late_initcall in place and just
add the additional reseed from entropy_credit_bits.
I would also repace the net_random() calls with secure_ipv4/6_port_ephemeral.
Still need to check if I have all the needed input available to those
functions available.
Greetings,
Hannes
^ permalink raw reply
* Re: xen-netback stable backport requests
From: David Miller @ 2013-11-09 20:56 UTC (permalink / raw)
To: Ian.Campbell; +Cc: xen-devel, netdev, stable
In-Reply-To: <1383826530.32399.13.camel@kazak.uk.xensource.com>
From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Thu, 7 Nov 2013 12:15:30 +0000
> 279f438e36c0 "xen-netback: Don't destroy the netdev until the vif is
> shut down" has hit stable but unfortunately is causing regressions.
>
> This should be fixed by dc62ccaccfb1 "xen-netback: transition to CLOSED
> when removing a VIF" which depends on ea732dff5cfa "xen-netback: Handle
> backend state transitions in a more robust way".
>
> Please could you queue hose two for stable.
Queued up for 3.10 and 3.11 -stable, thanks!
^ permalink raw reply
* Re: xen-netback regression in 3.10.18
From: David Miller @ 2013-11-09 20:57 UTC (permalink / raw)
To: david.vrabel; +Cc: netdev, stable, paul.durrant, ian.campbell, wei.liu2
In-Reply-To: <527B8465.6050901@citrix.com>
From: David Vrabel <david.vrabel@citrix.com>
Date: Thu, 7 Nov 2013 12:15:33 +0000
> 3.10.18 included 279f438e36c0 (xen-netback: Don't destroy the netdev
> until the vif is shut down) but this has a regression that was fixed by
> dc62ccaccfb1 (xen-netback: transition to CLOSED when removing a VIF)
>
> dc62ccaccfb1 depends on ea732dff5cfa (xen-netback: Handle backend state
> transitions in a more robust way) which is also a bug fix for certain
> Windows frontend drivers and is thus also a stable candidate.
>
> Dave can you ensure these two commits are tagged for the next 3.10.y
> stable release?
>
> ea732dff5cfa10789007bf4a5b935388a0bb2a8f
> dc62ccaccfb139d9b04bbc5a2688a4402adbfab3
Ian already asked me to do this, and the commits are necessary for
3.11 -stable as well.
They've been queued up.
^ permalink raw reply
* Re: [PATCH 2/2 net-next] openvswitch: Use skb_zerocopy() for upcall
From: Ben Hutchings @ 2013-11-09 22:02 UTC (permalink / raw)
To: Thomas Graf; +Cc: jesse, davem, dev, netdev, eric.dumazet
In-Reply-To: <63f4476190ef4044f7c09b225b4e93cd929bf0be.1383901577.git.tgraf@suug.ch>
On Fri, 2013-11-08 at 10:15 +0100, Thomas Graf wrote:
> Use of skb_zerocopy() avoids the expensive call to memcpy() when
> copying the packet data into the Netlink skb. Completes checksum
> through skb_checksum_help() if needed.
>
> Netlink messaged must be properly padded and aligned to meet
> sanity checks of the user space counterpart.
>
> Cost of memcpy is significantly reduced from:
> + 7.48% vhost-8471 [k] memcpy
> + 5.57% ovs-vswitchd [k] memcpy
> + 2.81% vhost-8471 [k] csum_partial_copy_generic
>
> to:
> + 5.72% ovs-vswitchd [k] memcpy
> + 3.32% vhost-5153 [k] memcpy
> + 0.68% vhost-5153 [k] skb_zerocopy
>
> (megaflows disabled)
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
> net/openvswitch/datapath.c | 52 +++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 1408adc..3f170e3 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
[...]
> @@ -441,13 +449,43 @@ static int queue_userspace_packet(struct net *net, int dp_ifindex,
> nla_len(upcall_info->userdata),
> nla_data(upcall_info->userdata));
>
> - nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
> + /* Only reserve room for attribute header, packet data is added
> + * in skb_zerocopy() */
> + if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0)))
> + goto out;
> + nla->nla_len = nla_attr_size(skb->len);
>
> - skb_copy_and_csum_dev(skb, nla_data(nla));
> + skb_zerocopy(user_skb, skb, skb->len, hlen);
>
> - genlmsg_end(user_skb, upcall);
> - err = genlmsg_unicast(net, user_skb, upcall_info->portid);
> + /* OVS user space expects the size of the message to be aligned to
> + * NLA_ALIGNTO. Aligning nlmsg_len is not enough, the actual bytes
> + * read must match nlmsg_len.
> + */
> + plen = NLA_ALIGN(user_skb->len) - user_skb->len;
> + if (plen > 0) {
> + int nr_frags = skb_shinfo(user_skb)->nr_frags;
> +
> + if (nr_frags) {
> + skb_frag_t *frag;
> +
> + frag = &skb_shinfo(user_skb)->frags[nr_frags -1];
> + skb_frag_size_add(frag, plen);
It looks like this is effectively padding with whatever happens to
follow the original packet content. This could result in a small
information leak. If the fragment has non-zero offset and already
extends to the end of a page, this could result in a segfault as the
next page may be unmapped.
Perhaps you could add the padding as an extra fragment pointing to a
preallocated zero page. If the skb already has the maximum number of
fragments, you would have to copy the last fragment in order to add
padding.
> + BUG_ON(frag->size > PAGE_SIZE);
[...]
I'm not sure that's a reasonable assumption either. We certainly allow
fragments to be larger than PAGE_SIZE in the transmit path.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Urgent
From: Vaishaliben Patel @ 2013-11-09 23:26 UTC (permalink / raw)
--
Good day, my name is Anna Cane (Mrs.),Did you receive my last e-mail
regarding my urgent need of your assistance from you? If not kindly
notify me via <showlilkindness@hotmail.com> for more details.
Anna Cane (Mrs.)
^ permalink raw reply
* Re: [PATCH net-next] nfnetlink: do not ack malformed messages
From: Cong Wang @ 2013-11-10 1:46 UTC (permalink / raw)
To: netdev
In-Reply-To: <527C1C38.40908@cogentembedded.com>
On Thu, 07 Nov 2013 at 23:03 GMT, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
>> --- a/net/netfilter/nfnetlink.c
>> +++ b/net/netfilter/nfnetlink.c
>> @@ -363,13 +363,15 @@ static void nfnetlink_rcv(struct sk_buff *skb)
>> struct net *net = sock_net(skb->sk);
>> int msglen;
>>
>> - if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>> - return netlink_ack(skb, nlh, -EPERM);
>> -
>
> Hm, how it even compiled before your patch?
>
You can return void inside a void function.
^ permalink raw reply
* Re: [PATCH net-next v2 6/10] bonding: rebuild the lock use for bond_activebackup_arp_mon()
From: Ding Tianhong @ 2013-11-10 4:08 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, David S. Miller,
Veaceslav Falico, Netdev
In-Reply-To: <527D0AEF.1060802@redhat.com>
>> + rcu_read_unlock();
>>
>> re_arm:
>> if (bond->params.arp_interval)
>> queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>>
>> - read_unlock(&bond->lock);
>> -
>> if (should_notify_peers) {
>> if (!rtnl_trylock())
>> return;
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index deb9738..90b745c 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -97,6 +97,13 @@
>> netdev_adjacent_get_private(bond_slave_list(bond)->prev) : \
>> NULL)
>>
>> +#define bond_first_slave_rcu(bond) \
>> + ({struct list_head *__ptr = (bond_slave_list(bond)); \
>> + struct list_head *__next = ACCESS_ONCE(__ptr->next); \
>> + likely(__ptr != __next) ? \
>> + netdev_adjacent_get_private_rcu(__next) : NULL; \
>> + })
>> +
> Honestly, I don't like this, it sure can be re-written in a more
> straight-forward manner.
I have re-write the function by 2 ways, the first one just like
list_first_or_null_rcu,
the second one just used the exist function
netdev_lower_get_next_private_rcu,
I think the first one is better, it is more exactly.
1:
+#define bond_first_slave_rcu(bond) \
+ ({struct list_head *__ptr = (bond_slave_list(bond)); \
+ struct list_head *__next = ACCESS_ONCE(__ptr->next); \
+ likely(__ptr != __next) ? \
+ (list_entry_rcu(__next, struct netdev_adjacent, list))->private : NULL; \
+ })
+
2:
+#define bond_first_slave_rcu(bond) \
+ ({struct list_head *iter = (bond_slave_list(bond)); \
+ netdev_lower_get_next_private_rcu(bond->dev, &iter) ? : NULL; \
+ })
what do you think about is, maybe you have more wonderful idea, pls
remind me,
thanks
Regards
Ding
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next 3/8] bonding: add downdelay netlink support
From: Scott Feldman @ 2013-11-10 7:28 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Veaceslav Falico, andy, netdev, Shrijeet Mukherjee
In-Reply-To: <14955.1383945357@death.nxdomain>
On Nov 8, 2013, at 11:15 AM, Jay Vosburgh <fubar@us.ibm.com> wrote:
> Scott Feldman <sfeldma@cumulusnetworks.com> wrote:
>
>> What I’d like to propose, and I hope that you’ll agree, is we tackle
>> this in two phases. The first phase is to finish the current
>> duplication effort to enable netlink equivalents of the attributes in
>> sysfs. This is a fairly mechanical process, preserving existing
>> behavior as you point out, and keeps the patches single-minded and easy
>> to review.
>>
>> The second phase is to revisit ordering constraints and find places
>> where we can remove constraints or streamline the dependency checks.
>>
>> Sound like a plan?
>
> Well, maybe.
>
> What I want to avoid for the iproute / netlink bonding support
> is all of the hoops that the initscripts / sysconfig scripts had to jump
> through to obey the current sysfs ordering limtations.
>
> The primary user of this is going to be iproute. Will the above
> quoted limitations (and their equivalents for various other options)
> make any difference with regards to the following:
>
> ip [...] bond arp_validate all arp_interval 1000
> ip [...] bond arp_interval 1000 arp_validate all
>
> I.e., does the ordering matter at the iproute level when
> multiple options are specified simultaneously?
Surprisingly, order doesn’t matter. Regardless of the ordering of attrs within the netlink msg, the processing order of attrs is always the same. In this case of bonding, it’s bond_netlink.c:bond_changelink() that dictates the order attrs are processed. So in your examples above, both versions would yield the same results.
> What happens if I do:
>
> ip [...] bond arp_interval 1000 arp_validate all miimon 100
>
> This is separate from simply changing one thing at a time, e.g.,
>
> ip [...] bond arp_validate all
> ip [...] bond arp_interval 1000
This is the problem to be solved. Bonding has several mutually exclusive attrs. When mutually exclusive options are presented at the same time (like in you first example above), we can pick a winner using the processing order in bond_changelink(). When single attrs are set (like in second example), it’s trickier to pick winner in bond_changelink(), but do-able, I think.
> will presuambly hit the test above, and this is where the
> ordering stuff comes into play for sure.
>
> Also, as I look at the iproute patch, it doesn't appear to
> accept the text names for the options, only numeric values (e.g., "ip
> [...] bond arp_validate 3"). That appears to be a limitation of Jiri's
> original iproute patch as well. Am I the only person that perfers the
> text labels (e.g., arp_validate as "all") to numbers (arp_validate as
> "3")?
I can add text names to the iproute patch for v2.
> So, honestly, I think if the ordering constraints are going to
> be relaxed, it should happen sooner rather than later. Perhaps not in
> the same patch as the netlink support, but ideally at least in the same
> series, so there is no real release with the constraints. Changing the
> constraints after the script, etc, conversion is done doesn't really
> help much.
Ok, let me study this some more before sending v2.
-scott
^ permalink raw reply
* Dear Sir/Madam
From: Allen and Violet Large @ 2013-11-10 7:41 UTC (permalink / raw)
To: Recipients
I saw your email address during the course of my research today.My Name is Allen my wife and I won a Jackpot Lottery 11.3 million in july and during the process my wife passed away as a result of cancer illness, we are donating the sum of 1.million dollars to 6 lucky individual over the world and if you received this email then you are one of the lucky recipients and all you have to do is get back to us so that we can send your details to the payout bank.
Please note that you have to contact my private email for more information: allenand.viole@aol.com
You can verify this by visiting the web pages below.
http://www.dailymail.co.uk/news/article-1326473/Canadian-couple-Allen-Violet-Large-away-entire-11-2m-lottery-win.html
Goodluck,
Allen and Violet Large
^ permalink raw reply
* Dear Sir/Madam
From: Allen and Violet Large @ 2013-11-10 7:41 UTC (permalink / raw)
To: Recipients
I saw your email address during the course of my research today.My Name is Allen my wife and I won a Jackpot Lottery 11.3 million in july and during the process my wife passed away as a result of cancer illness, we are donating the sum of 1.million dollars to 6 lucky individual over the world and if you received this email then you are one of the lucky recipients and all you have to do is get back to us so that we can send your details to the payout bank.
Please note that you have to contact my private email for more information: allenand.viole@aol.com
You can verify this by visiting the web pages below.
http://www.dailymail.co.uk/news/article-1326473/Canadian-couple-Allen-Violet-Large-away-entire-11-2m-lottery-win.html
Goodluck,
Allen and Violet Large
^ permalink raw reply
* Panic on 3.10.18 in nf_conntrack_sip with IPv6
From: Chris Boot @ 2013-11-10 10:55 UTC (permalink / raw)
To: netdev, linux-kernel, netfilter-devel, netfilter, coreteam
Hi folks,
I seem to have a pretty reliably reproducible panic on a fairly plain
3.10.18 kernel when routing IPv6 SIP packets.
The setup is a VM server that routes IPv6 from its external interface to
the VMs running on the host. The host is running Shorewall-generated
ip6tables.
All it takes for this to occur is for my Snom phone running beta IPv6
firmware to attempt to make a call to the Asterisk instance running
inside a VM on the host. This will take the host down and all the VMs
running on it, so I'm a bit reluctant to keep testing this.
Interestingly, I have a 32-bit x86 router running the exact same kernel
with the same modules, and passing the same packets through it doesn't
crash that box. I'll admit the configuration isn't all that similar, but
I thought I should mention it anyway.
Any help in reproducing, diagnosing and fixing this is much appreciated.
Panic below (sorry, I couldn't catch the first few lines):
> nf_nat_sip nf_nat_pptp nf_nat_proto_gre nf_nat_irc nf_nat_h323 nf_nat_ftp nf_nat_amanda nf_conntrack_sane nf_conntrack_tftp nf_conntrack_sip ts_kmp nf_conntrack_proto_udplite nf_conntrack_proto_sctp nf_conntrack_pptp nf_conntrack_proto_gre nf_conntrack_netlink nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_irc nf_conntrack_h323 nf_conntrack_amanda nf_conntrack_ftp xt_time xt_TPROXY xt_TCPMSS nf_tproxy_core xt_sctp xt_tcpmss xt_policy xt_pkttype xt_physdev xt_owner xt_NFLOG nfnetlink_log xt_NFQUEUE xt_multiport xt_mark xt_mac xt_limit xt_length xt_iprange xt_helper xt_hashlimit xt_DSCP xt_dscp xt_dccp xt_connmark xt_CLASSIFY xt_AUDIT iptable_nat nf_nat_ipv4 nf_nat ip6t_REJECT xt_tcpudp xt_state nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack_ipv4 nf_defrag_ipv4 nf
_conntrack ip6table_raw ip6table_mangle iptable_mangle nfnetlink ip6table_filter ip6_tables iptable_filter ip_tables x_tables bridge stp llc bonding w83627ehf hwmon_vid sha1_ssse3 sha1_generic ipmi
_poweroff ipmi_devintf ipmi_si ipmi_msghandler vhost_net tun macvtap macvlan drbd(O) libcrc32c loop coretemp kvm_intel kvm crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul snd_pcm snd_page_alloc snd_timer glue_helper ablk_helper cryptd iTCO_wdt mperf iTCO_vendor_support snd psmouse soundcore lpc_ich serio_raw i2c_i801 pcspkr joydev evdev mfd_core processor thermal_sys button microcode ext4 crc16 jbd2 mbcache dm_mod raid1 md_mod sg sd_mod crc_t10dif usb_storage hid_generic usbhid hid igb ahci i2c_algo_bit ehci_pci i2c_core libahci ehci_hcd e1000e dca libata usbcore ptp usb_common scsi_mod pps_core
> [ 798.824659] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 3.10.18-0.bootc.1-amd64 #1 bootc 3.10.18-1~bootc1
> [ 798.882711] Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS 1.1a 09/28/2011
> [ 798.913159] task: ffffffff81613400 ti: ffffffff81600000 task.ti: ffffffff81600000
> [ 798.940448] RIP: 0010:[<ffffffff812c5b22>] [<ffffffff812c5b22>] pskb_expand_head+0x2a/0x1e1
> [ 798.968806] RSP: 0018:ffff88043fc037c0 EFLAGS: 00010202
> [ 798.996088] RAX: 0000000000000002 RBX: ffff88041ad4fb00 RCX: 0000000000000020
> [ 799.022024] RDX: 00000000000007ab RSI: 0000000000000000 RDI: ffff88041ad4fb00
> [ 799.048233] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff88043fc03938
> [ 799.075704] R10: ffffffff8169b3d0 R11: 0000000000000000 R12: ffff8804190f09c0
> [ 799.416456] R13: 0000000000000028 R14: 000000000000010d R15: ffffffff81344366
> [ 799.442700] FS: 0000000000000000(0000) GS:ffff88043fc00000(0000) knlGS:0000000000000000
> [ 799.470328] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 799.494800] CR2: 000000000090f340 CR3: 000000000160c000 CR4: 00000000000407f0
> [ 799.519552] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 799.545223] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 799.569433] Stack:
> [ 799.588302] 0000000000000030 ffff88041ad4fb00 0000000000000030 ffff8804190f09c0
> [ 799.613649] 0000000000000028 000000000000010d ffffffff81344366 ffffffff812c633a
> [ 799.636945] 0000000000000028 ffff88041ad4fb00 0000000000000030 ffff8804190f09c0
> [ 799.667011] Call Trace:
> [ 799.691674] <IRQ>
> [ 799.693796] [<ffffffff81344366>] ? ip6_fragment+0x8b5/0x8b5
> [ 799.730242] [<ffffffff812c633a>] ? __pskb_pull_tail+0x68/0x254
> [ 799.751162] [<ffffffff81344366>] ? ip6_fragment+0x8b5/0x8b5
> [ 799.772907] [<ffffffffa056e510>] ? sip_help_udp+0x69/0x95 [nf_conntrack_sip]
> [ 799.794260] [<ffffffffa04744cb>] ? ipv6_helper+0xa7/0xb2 [nf_conntrack_ipv6]
> [ 799.815274] [<ffffffff812f201a>] ? nf_iterate+0x42/0x80
> [ 799.835591] [<ffffffff81341654>] ? __ipv6_neigh_lookup_noref+0x95/0x95
> [ 799.857404] [<ffffffff812f20c1>] ? nf_hook_slow+0x69/0x100
> [ 799.878454] [<ffffffff81344366>] ? ip6_fragment+0x8b5/0x8b5
> [ 799.898988] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 799.919895] [<ffffffff813419ac>] ? nf_hook_thresh.constprop.36+0x2e/0x33
> [ 799.941898] [<ffffffff813419ac>] ? nf_hook_thresh.constprop.36+0x2e/0x33
> [ 799.963582] [<ffffffff81344437>] ? ip6_output+0x7a/0x83
> [ 799.983090] [<ffffffff81343a10>] ? ip6_forward+0x5fd/0x69e
> [ 800.001437] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.019612] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.036639] [<ffffffffa04746ac>] ? __ipv6_conntrack_in+0xc4/0x13f [nf_conntrack_ipv6]
> [ 800.057257] [<ffffffff812f201a>] ? nf_iterate+0x42/0x80
> [ 800.075044] [<ffffffff812f20c1>] ? nf_hook_slow+0x69/0x100
> [ 800.092089] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.108860] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.353154] [<ffffffffa046bc5a>] ? nf_ct_frag6_output+0x9f/0xe8 [nf_defrag_ipv6]
> [ 800.371387] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.387677] [<ffffffffa046b0bc>] ? ipv6_defrag+0xbb/0xcf [nf_defrag_ipv6]
> [ 800.406280] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.424998] [<ffffffff812f201a>] ? nf_iterate+0x42/0x80
> [ 800.441318] [<ffffffff812f20c1>] ? nf_hook_slow+0x69/0x100
> [ 800.457694] [<ffffffff8134446d>] ? pskb_may_pull+0x2d/0x2d
> [ 800.474649] [<ffffffff813445b9>] ? nf_hook_thresh.constprop.13+0x34/0x39
> [ 800.495046] [<ffffffff81344b43>] ? ipv6_rcv+0x2bb/0x30b
> [ 800.511896] [<ffffffff812cea5d>] ? __netif_receive_skb_core+0x437/0x4af
> [ 800.532539] [<ffffffff812ceca1>] ? netif_receive_skb+0x42/0x73
> [ 800.551414] [<ffffffff812cf419>] ? napi_gro_receive+0x35/0x76
> [ 800.568152] [<ffffffffa012e20b>] ? e1000_clean_rx_irq+0x249/0x2cb [e1000e]
> [ 800.589151] [<ffffffffa0131698>] ? e1000e_poll+0x65/0x203 [e1000e]
> [ 800.606255] [<ffffffff810742f4>] ? ktime_get+0x5f/0x6b
> [ 800.622019] [<ffffffff812cf1b8>] ? net_rx_action+0xa7/0x1d9
> [ 800.640555] [<ffffffff8139238c>] ? _raw_spin_unlock_irqrestore+0xc/0xd
> [ 800.658116] [<ffffffff812730de>] ? add_interrupt_randomness+0x39/0x16f
> [ 800.677242] [<ffffffff8104244a>] ? __do_softirq+0xe4/0x1f9
> [ 800.696819] [<ffffffff81398bdc>] ? call_softirq+0x1c/0x30
> [ 800.713258] [<ffffffff8100e9ee>] ? do_softirq+0x3a/0x78
> [ 800.731145] [<ffffffff8104262a>] ? irq_exit+0x3f/0x83
> [ 800.747466] [<ffffffff8100e6ff>] ? do_IRQ+0x81/0x97
> [ 800.763133] [<ffffffff8139262d>] ? common_interrupt+0x6d/0x6d
> [ 800.780351] <EOI>
> [ 800.782499] [<ffffffff81078ffb>] ? clockevents_program_event+0x9a/0xb6
> [ 800.813469] [<ffffffff812a8110>] ? arch_local_irq_enable+0x4/0x8
> [ 800.831484] [<ffffffff812a84db>] ? cpuidle_enter_state+0x46/0xb1
> [ 800.849729] [<ffffffff812a8615>] ? cpuidle_idle_call+0xcf/0x126
> [ 800.869185] [<ffffffff81013b3b>] ? arch_cpu_idle+0x6/0x1a
> [ 800.885493] [<ffffffff81073255>] ? cpu_startup_entry+0x106/0x169
> [ 800.902532] [<ffffffff816b5d40>] ? start_kernel+0x3d7/0x3e2
> [ 800.922455] [<ffffffff816b577f>] ? repair_env_string+0x57/0x57
> [ 800.939302] [<ffffffff816b559a>] ? x86_64_start_kernel+0xf2/0xfd
> [ 800.956528] Code: c3 41 57 41 56 41 55 41 54 55 53 48 89 fb 55 8b 87 dc 00 00 00 89 f5 01 f0 01 c2 85 f6 79 02 0f 0b 8b 87 f4 00 00 00 ff c8 74 02 <0f> 0b 83 c2 3f 89 c8 41 89 cd 80 cc 20 83 e2 c0 f6 87 b2 00 00
> [ 801.015687] RIP [<ffffffff812c5b22>] pskb_expand_head+0x2a/0x1e1
> [ 801.034404] RSP <ffff88043fc037c0>
> [ 801.049813] ---[ end trace a0ea98f51afb8cc0 ]---
> [ 801.454124] Kernel panic - not syncing: Fatal exception in interrupt
> [ 801.474385] Rebooting in 120 seconds..
The O taint is due to loading LinBIT's drbd module. The crash occurs
even without this, and also in a 3.7.10 kernel that I was using before.
Cheers,
Chris
--
Chris Boot
bootc@bootc.net
^ permalink raw reply
* Dear Sir/Madam
From: Allen and Violet Large @ 2013-11-10 10:05 UTC (permalink / raw)
To: Recipients
I saw your email address during the course of my research today.My Name is Allen my wife and I won a Jackpot Lottery 11.3 million in july and during the process my wife passed away as a result of cancer illness, we are donating the sum of 1.million dollars to 6 lucky individual over the world and if you received this email then you are one of the lucky recipients and all you have to do is get back to us so that we can send your details to the payout bank.
Please note that you have to contact my private email for more information: allenand.viole@aol.com
You can verify this by visiting the web pages below.
http://www.dailymail.co.uk/news/article-1326473/Canadian-couple-Allen-Violet-Large-away-entire-11-2m-lottery-win.html
Goodluck,
Allen and Violet Large
^ permalink raw reply
* [PATCH net-next FIX] RDMA/cma: Fix build breakage when infiniband is built-in
From: Or Gerlitz @ 2013-11-10 13:28 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, Eyal Perry, Or Gerlitz
From: Eyal Perry <eyalpe@mellanox.com>
Commit eb072c4 ("RDMA/cma: Set IBoE SL (user-priority) by egress map when using
vlans"), broke the build when CONFIG_VLAN_8021Q=m and CONFIG_INFINIBAND=y,
because the rdma_cm module attempted to call vlan_dev_get_egress_qos_mask()
which in that case is a modular code being called from built-in kernel code.
Fix this by adding a Kconfig dependency to prevent the rdma_cm code from being
built in when 8021Q is built modular, in a similar manner we do with IPv6.
Signed-off-by: Eyal Perry <eyalpe@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
Dave, this fixes an issue with the mentioned commit which is in net-next
drivers/infiniband/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index b84791f..079dfe4 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -51,6 +51,7 @@ config INFINIBAND_ADDR_TRANS
bool
depends on INET
depends on !(INFINIBAND = y && IPV6 = m)
+ depends on !(INFINIBAND = y && VLAN_8021Q = m)
default y
source "drivers/infiniband/hw/mthca/Kconfig"
--
1.7.1
^ permalink raw reply related
* [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Arnaud Ebalard @ 2013-11-10 13:53 UTC (permalink / raw)
To: Eric Dumazet, David S. Miller, Greg Kroah-Hartman
Cc: netdev, stable, linux-arm-kernel
Hi,
I decided to upgrade the kernel on one of my ReadyNAS 102 from 3.11.1 to
3.11.7. The device is based on Marvell Armada 370 SoC and uses mvneta
driver. Mine runs Debian armel unstable but I can confirm the issue also
happens on a debian harmhf unstable.
Doing some scp transfers of files located on the NAS (1000baseT-FD on
both side), I noticed the transfers rate is ridiculously slow (280KB/s).
I did the same test with a 3.12 kernel and got the same results,
i.e. AFAICT the bug also exist upstream.
So, I decided to go to hell and start digging a bit: I run a 'git bisect'
session on stable tree from 3.11.1 (known good) to 3.11.7 (known
bad). The results are given below.
I decided to reboot on my old 3.11.1 kernel and do 20 files transfers
of a 1GB file located on the NAS to my laptop via scp. I took the 20+
minutes and let them all finish: each transfer took between 1min5s and
1min7s (around 16MB/s, the limitation in that case being the crypto part).
I rebooted again and did the exact same thing on the 3.11.7 and after
the completion of the first file transfer in 1m6s (16MB/s), the second
one gave me that:
arno@small:~scp RN102:/tmp/random /dev/null
random 0% 1664KB 278.9KB/s 1:05:37 ETA^C
And it continued that way for the remaining transfers (i did ^c after
some seconds to restart the transfer when the rate was low):
$ for k in $(seq 1 20) ; do scp RN102:random /dev/null ; done
random 100% 1024MB 15.6MB/s 01:06 ETA^C
random 0% 9856KB 282.2KB/s 1:01:20 ETA^C
random 16% 168MB 563.9KB/s 25:54 ETA^C
random 0% 2816KB 273.5KB/s 1:03:43 ETA^C
random 100% 1024MB 15.5MB/s 01:06
random 1% 17MB 282.3KB/s 1:00:54 ETA^C
random 0% 544KB 259.2KB/s 1:07:23 ETA^C
random 0% 4224KB 277.3KB/s 1:02:45 ETA^C
random 0% 832KB 262.1KB/s 1:06:37 ETA^C
random 0% 3360KB 273.4KB/s 1:03:43 ETA^C
random 0% 3072KB 271.8KB/s 1:04:07 ETA^C
random 0% 832KB 262.1KB/s 1:06:37 ETA^C
random 0% 1408KB 267.0KB/s 1:05:21 ETA^C
random 0% 1120KB 264.7KB/s 1:05:57 ETA
...
To be sure, I did 2 additional reboots, one on each kernel and the
result is consistent. Perfect on 3.11.1 and slow rate most of the time
on 3.11.7 (Both kernel are compiled from a fresh make clean, using the
same config file).
Then, knowing that, I started a git bisect session on stable tree to end
up with the following suspects. I failed to go any further to a single
commit, due to crashes, but I could recompile a kernel w/ debug info and
report what I get if neeeded.
commit dc0791aee672 tcp: do not forget FIN in tcp_shifted_skb() [bad]
commit 18ddf5127c9f tcp: must unclone packets before mangling them
commit 80bd5d8968d8 tcp: TSQ can use a dynamic limit
commit dbeb18b22197 tcp: TSO packets automatic sizing
commit 50704410d014 Linux 3.11.6 [good]
Eric, David, if it has already been reported and fixed, just tell
me. Otherwise, if you have any ideas, I'll be happy to test this
evening.
Cheers,
a+
Just in case it may be useful, here is what ethtool reports on RN102:
# ethtool -i eth0
driver: mvneta
version: 1.0
firmware-version:
bus-info: eth0
supports-statistics: no
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
# ethtool -k eth0
Features for eth0:
rx-checksumming: off [fixed]
tx-checksumming: on
tx-checksum-ipv4: on
tx-checksum-ip-generic: off [fixed]
tx-checksum-ipv6: off [fixed]
tx-checksum-fcoe-crc: off [fixed]
tx-checksum-sctp: off [fixed]
scatter-gather: on
tx-scatter-gather: on
tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
tx-tcp-segmentation: off [fixed]
tx-tcp-ecn-segmentation: off [fixed]
tx-tcp6-segmentation: off [fixed]
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: off [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-mpls-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: on
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-10 14:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383884730.9412.175.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 07, 2013 at 08:25:30PM -0800, Eric Dumazet wrote:
> On Fri, 2013-11-08 at 11:59 +0800, Herbert Xu wrote:
>
> > However, I still have one reason for preferring my patch, it'll
> > be easier to prodce TSO packets with it. Let me see if I can
> > fix up the arbitrary frag boundary issue without making it too
> > ugly.
>
> Sure !
I ended up giving up on the recursion idea and borrowed your
iterative approach. I haven't yet had the chance to test it
yet but here is the WIP.
The main assumptions are that virtio_net frag_list is always non-
linear and GRO frag_list may only contain a linear head part that
is exactly MSS bytes long.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3735fad..fab44ff 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2776,6 +2776,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
struct sk_buff *segs = NULL;
struct sk_buff *tail = NULL;
struct sk_buff *fskb = skb_shinfo(skb)->frag_list;
+ skb_frag_t *skb_frag = skb_shinfo(skb)->frags;
unsigned int mss = skb_shinfo(skb)->gso_size;
unsigned int doffset = skb->data - skb_mac_header(skb);
unsigned int offset = doffset;
@@ -2815,16 +2816,23 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
if (hsize > len || !sg)
hsize = len;
- if (!hsize && i >= nfrags) {
- BUG_ON(fskb->len != len);
+ if (!hsize && i >= nfrags && skb_headlen(fskb)) {
+ BUG_ON(skb_headlen(fskb) != len);
pos += len;
+ i = 0;
+ nfrags = skb_shinfo(fskb)->nr_frags;
+ skb_frag = skb_shinfo(fskb)->frags;
+
nskb = skb_clone(fskb, GFP_ATOMIC);
fskb = fskb->next;
if (unlikely(!nskb))
goto err;
+ if (unlikely(pskb_trim(nskb, len)))
+ goto err;
+
hsize = skb_end_offset(nskb);
if (skb_cow_head(nskb, doffset + headroom)) {
kfree_skb(nskb);
@@ -2861,7 +2869,8 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
nskb->data - tnl_hlen,
doffset + tnl_hlen);
- if (fskb != skb_shinfo(skb)->frag_list)
+ if (fskb != skb_shinfo(skb)->frag_list &&
+ nskb->len == len + doffset)
goto perform_csum_check;
if (!sg) {
@@ -2879,8 +2888,20 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
skb_shinfo(nskb)->tx_flags = skb_shinfo(skb)->tx_flags & SKBTX_SHARED_FRAG;
- while (pos < offset + len && i < nfrags) {
- *frag = skb_shinfo(skb)->frags[i];
+ while (pos < offset + len) {
+ if (i >= nfrags) {
+ BUG_ON(skb_headlen(fskb));
+
+ i = 0;
+ nfrags = skb_shinfo(fskb)->nr_frags;
+ skb_frag = skb_shinfo(fskb)->frags;
+
+ BUG_ON(!nfrags);
+
+ fskb = fskb->next;
+ }
+
+ *frag = *skb_frag;
__skb_frag_ref(frag);
size = skb_frag_size(frag);
@@ -2891,37 +2912,26 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
skb_shinfo(nskb)->nr_frags++;
- if (pos + size <= offset + len) {
- i++;
- pos += size;
- } else {
- skb_frag_size_sub(frag, pos + size - (offset + len));
- goto skip_fraglist;
+ if (pos + size >= offset + len) {
+ skb_frag_size_sub(frag,
+ pos + size - (offset + len));
+ break;
}
+ skb_frag++;
+ i++;
+ pos += size;
frag++;
- }
-
- if (pos < offset + len) {
- struct sk_buff *fskb2 = fskb;
- BUG_ON(pos + fskb->len != offset + len);
-
- pos += fskb->len;
- fskb = fskb->next;
-
- if (fskb2->next) {
- fskb2 = skb_clone(fskb2, GFP_ATOMIC);
- if (!fskb2)
- goto err;
- } else
- skb_get(fskb2);
-
- SKB_FRAG_ASSERT(nskb);
- skb_shinfo(nskb)->frag_list = fskb2;
+ if (unlikely(skb_shinfo(nskb)->nr_frags >=
+ MAX_SKB_FRAGS)) {
+ net_warn_ratelimited(
+ "skb_segment: too many frags: %u %u\n",
+ pos, mss);
+ goto err;
+ }
}
-skip_fraglist:
nskb->data_len = len - hsize;
nskb->len += nskb->data_len;
nskb->truesize += nskb->data_len;
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH net-next] xfrm: check function pointer of xfrm_mgr before use it
From: baker.kernel @ 2013-11-10 14:25 UTC (permalink / raw)
To: herbert, davem, steffen.klassert; +Cc: netdev, baker.zhang
From: "baker.zhang" <baker.kernel@gmail.com>
Signed-off-by: baker.zhang <baker.kernel@gmail.com>
---
For current kernel source, there is no problem.
In our vpn product, we need a xfrm_km in kernel module
to monitor the xfrm state change.
thus, the 'acquire' and 'compile_policy' may be NULL.
So I think we should do the check before use it.
net/xfrm/xfrm_state.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index b9c3f9e..541f684 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1679,9 +1679,11 @@ int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
rcu_read_lock();
list_for_each_entry_rcu(km, &xfrm_km_list, list) {
- acqret = km->acquire(x, t, pol);
- if (!acqret)
- err = acqret;
+ if (km->acquire) {
+ acqret = km->acquire(x, t, pol);
+ if (!acqret)
+ err = acqret;
+ }
}
rcu_read_unlock();
return err;
@@ -1783,10 +1785,12 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
err = -EINVAL;
rcu_read_lock();
list_for_each_entry_rcu(km, &xfrm_km_list, list) {
- pol = km->compile_policy(sk, optname, data,
- optlen, &err);
- if (err >= 0)
- break;
+ if (km->compile_policy) {
+ pol = km->compile_policy(sk, optname, data,
+ optlen, &err);
+ if (err >= 0)
+ break;
+ }
}
rcu_read_unlock();
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v3] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2013-11-10 17:57 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, wg, linux-can, linux-sh, vksavl
In-Reply-To: <527D6B88.3010409@cogentembedded.com>
[-- Attachment #1: Type: text/plain, Size: 7271 bytes --]
On 11/08/2013 11:54 PM, Sergei Shtylyov wrote:
[...]
>>>> Please use a common prefix for all defines.
>
>>> OK, done now. Could you however explain why the file-local #define's
>>> should be prefixed? It's not quite obvious to me...
>
>> It's about readability and maintainability. If you don't know the
>> driver, but all driver local defines and functions have a common prefix,
>> it's much easier to read if you are not the author of the driver IMHO.
>
> Well, actually I think the last changes somewhat impaired the
> readability. My idea was that you want to exclude name conflicts with
> #include'd headers this way...
IMHO this way it's clear to everyone, that certain defines are
driver/hardware specific.
>>> [...]
>>>>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>>>>> + struct net_device *ndev)
>>>>> +{
>>>>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>>>>> + struct can_frame *cf = (struct can_frame *)skb->data;
>>>>> + u32 data, mier1, mbxno, i;
>>>>> + unsigned long flags;
>>>>> + u8 mctl = 0;
>>>>> +
>>>>> + if (can_dropped_invalid_skb(ndev, skb))
>>>>> + return NETDEV_TX_OK;
>>>>> +
>>>>> + spin_lock_irqsave(&priv->mier_lock, flags);
>>>>> + mier1 = readl(&priv->regs->mier1);
>>>>> + if (mier1) {
>>>>> + i = __builtin_clz(mier1);
>>>>> + mbxno = i ? N_MBX - i : FIRST_TX_MB;
>>>>> + } else {
>>>>> + mbxno = FIRST_TX_MB;
>>>>> + }
>
>>>> Can you explain how the hardware arbitration works, and you do you
>>>> guarantee the CAN frames are send by the hardware in the same order you
>>>> put them into the hardware.
>
>>> Tx mailbox with the smallest mailbox number has the highest
>>> priority.
>>> The other possible Tx mailbox selection rule (not used by the driver
>>> now) is ID priority transmit mode (as defined in the ISO 11898-1 specs).
>>> The algorithm used guarantees the mailboxes are filled sequentially.
>
> Well, not quite, unfortunately -- it wraps at the last mailbox...
Yes sure, that's the downside if there isn't a real FIFO in the
hardware. I suggest to have two pointers in your private struct:
unsinged int tx_head;
unsigned int tx_tail;
static inline unsigned int get_tx_head_mb(struct *priv)
{
return priv->tx_head % RCAN_TX_QUEUE_SIZE;
}
static inline unsigned int get_tx_tail_mb(struct priv *priv)
{
return priv->tx_tail % RCAN_TX_QUEUE_SIZE;
}
Put the next CAN frame into get_tx_head_mb(), increment priv->tx_head
and stop your queue if all buffers are used or wrap around.
/* stop, if all buffers are used or wrap around */
if ((priv->tx_head - priv->tx_tail == RCAN_TX_QUEUE_SIZE) &&
get_tx_head_mb(priv) == 0)
netif_stop_queue(dev);
But I think, as you don't have any additional prio bits, it boils down to:
/* stop, if wrap around */
if (get_tx_head_mb(priv) == 0)
netif_stop_queue(dev);
In your tx-complete, use something like this:
reg = read_tx_complete();
for (/* nix */; priv->tx_head - priv->tx_tail > 0; priv->tx_tail++) {
mb = get_tx_tail_mb(priv);
if (!reg & (1 << mb))
break;
read_can_frame();
}
if ((get_tx_head_mb(priv) != 0) || (get_tx_tail_mb(priv) == 0))
netif_wake_queue(dev);
>> I see. You are using mier1 to track the used mailboxes....correct?
>
> Yes, the mailbox interrupts in MIER1 are enabled only for used
> mailboxes.
>
>>> + if (unlikely(mier1 == 0xffffffff))
>>> + netif_stop_queue(ndev);
>
>> Then you have a race condition in your tx-complete handler
>> rcar_can_tx_done(), as you call netif_wake_queue() unconditionally. If
>
> Yes, I'm seeing it now...
>
>> mier1 == 0xffffffff you must wait until _all_ mailboxes are transmitted
>
> That 0xffffffff criterion seems wrong for me now, I changed the
> algorithm and moved the criterion but didn't update it. The correct one
> seems to be:
>
> if (unlikely(mier1 & 0x80000000))
> netif_stop_queue(ndev);
Better handle it in software altogether as outlined above.
>> until you are allowed to reenable the mailboxes. Have a look at the
>> at91_can driver, it's using a similar scheme. The lowest mailbox is
>> transmitted first, but there are three additional bits that indicate the
>> priority.
>
> You mean 4 bits? Priorities are from 0 to 15...
Yes, 4 bits (0xf), but the algorithm isn't limited to 4 bits.
>>> I've used 'canfdtest' as suggested by Wolfgang Grandegger to verify, see
>>> the log below:
>
>>> root@am335x-evm:~# ./canfdtest -v -g can0
>>> interface = can0, family = 29, type = 3, proto = 1
>>> ...............................................................................C
>>>
>>>
>>> Test messages sent and received: 483203
>>> Exiting...
>>> root@am335x-evm:~#
>
> As you can see, 'canfdtest' didn't detect any race, maybe you could
> recommend a test which would help to detect it?
It's better to think to detect a race condition, then to test for it. I
suggest to use the algorithm outlined above, as implemented in the
at91_can.c driver.
>>> [...]
>>>>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>>>>> +{
>>>>> + struct rcar_can_priv *priv = container_of(napi,
>>>>> + struct rcar_can_priv, napi);
>>>>> + int num_pkts = 0;
>>>>> +
>>>>> + /* Find mailbox */
>>>>> + while (num_pkts < quota) {
>>>>> + u8 mctl, mbx;
>>>>> +
>>>>> + mbx = readb(&priv->regs->mssr);
>
>>>> How does the RX work? Is it a hardware FIFO?
>
>>> In short, the MSSR register provides the smallest Rx mailbox number
>>> that is looked up in the Rx search mode. We read MSSR until no search
>>> results can be obtained, so it is some sort of FIFO.
>
>> This looks racy....
>
> Could you please elaborate?
Consider the hardware fills mailboxes 0...8, then your rx_poll starts to
read mailbox 0, clear mailbox 0, read mb 1, clear mb 2, then another CAN
frame arrives. Which mailbox will be filled next? Which mailbox will be
read next by rx_poll?
>>> And there is separate FIFO operation mode: some mailboxes can be
>>> configured as FIFO and serviced by special registers but this operation
>>> mode is not supported by the driver.
>
>> if you hardware supports a real FIFO then I strongly suggest to make use
>> of it.
>
> Well, Tx/Rx FIFOs are only 4 frames deep (although I haven't seen
> more than 2 Rx mailboxes ready in a row, there are 32 Tx mailboxes);
> also FIFO mode is less documented than mailbox mode (hence some nasty
> surprises are possible). We still can use mailboxes when in FIFO mode,
> it's just 8 of them are reserved for Tx/Rx FIFOs.
Who will the current rx_poll behave in the above outlined situation?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] xfrm: check function pointer of xfrm_mgr before use it
From: Sergei Shtylyov @ 2013-11-10 19:03 UTC (permalink / raw)
To: baker.kernel, herbert, davem, steffen.klassert; +Cc: netdev
In-Reply-To: <1384093556-8987-1-git-send-email-baker.kernel@gmail.com>
Hello.
On 10-11-2013 18:25, baker.kernel@gmail.com wrote:
> From: "baker.zhang" <baker.kernel@gmail.com>
> Signed-off-by: baker.zhang <baker.kernel@gmail.com>
> ---
> For current kernel source, there is no problem.
> In our vpn product, we need a xfrm_km in kernel module
> to monitor the xfrm state change.
> thus, the 'acquire' and 'compile_policy' may be NULL.
> So I think we should do the check before use it.
> net/xfrm/xfrm_state.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index b9c3f9e..541f684 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
[...]
> @@ -1783,10 +1785,12 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
> err = -EINVAL;
> rcu_read_lock();
> list_for_each_entry_rcu(km, &xfrm_km_list, list) {
> - pol = km->compile_policy(sk, optname, data,
> - optlen, &err);
> - if (err >= 0)
> - break;
> + if (km->compile_policy) {
> + pol = km->compile_policy(sk, optname, data,
> + optlen, &err);
According the networking coding style, the continuation line should start
right under 'sk' on the previous line.
WBR, Sergei
^ permalink raw reply
* [PATCH] net: wireless: iwlwifi: remove minor dead code
From: Michal Nazarewicz @ 2013-11-10 19:06 UTC (permalink / raw)
To: Johannes Berg, Emmanuel Grumbach, John W. Linville
Cc: Intel Linux Wireless, linux-wireless, netdev, linux-kernel,
Michal Nazarewicz
From: Michal Nazarewicz <mina86@mina86.com>
inta is checked to be zero in a IRQ_NONE branch so afterwards it
cannot be zero as it is never modified.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/net/wireless/iwlwifi/pcie/rx.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
index 3f237b4..c32122a 100644
--- a/drivers/net/wireless/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
@@ -1121,7 +1121,6 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
struct iwl_trans *trans = data;
struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
u32 inta, inta_mask;
- irqreturn_t ret = IRQ_NONE;
lockdep_assert_held(&trans_pcie->irq_lock);
@@ -1150,7 +1149,13 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
* or due to sporadic interrupts thrown from our NIC. */
if (!inta) {
IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
- goto none;
+ /* re-enable interrupts here since we don't have anything to
+ * service. only Re-enable if disabled by irq and no
+ * schedules tasklet. */
+ if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
+ !trans_pcie->inta)
+ iwl_enable_interrupts(trans);
+ return IRQ_NONE;
}
if ((inta == 0xFFFFFFFF) || ((inta & 0xFFFFFFF0) == 0xa5a5a5a0)) {
@@ -1168,19 +1173,7 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
trans_pcie->inta |= inta;
/* the thread will service interrupts and re-enable them */
- if (likely(inta))
- return IRQ_WAKE_THREAD;
-
- ret = IRQ_HANDLED;
-
-none:
- /* re-enable interrupts here since we don't have anything to service. */
- /* only Re-enable if disabled by irq and no schedules tasklet. */
- if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
- !trans_pcie->inta)
- iwl_enable_interrupts(trans);
-
- return ret;
+ return IRQ_WAKE_THREAD;
}
/* interrupt handler using ict table, with this interrupt driver will
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] net: wireless: iwlwifi: remove minor dead code
From: Sergei Shtylyov @ 2013-11-10 19:15 UTC (permalink / raw)
To: Michal Nazarewicz, Johannes Berg, Emmanuel Grumbach,
John W. Linville
Cc: Intel Linux Wireless, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Michal Nazarewicz
In-Reply-To: <1384110397-24386-1-git-send-email-mpn-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Hello.
On 10-11-2013 23:06, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
> inta is checked to be zero in a IRQ_NONE branch so afterwards it
> cannot be zero as it is never modified.
> Signed-off-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/net/wireless/iwlwifi/pcie/rx.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
> diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
> index 3f237b4..c32122a 100644
> --- a/drivers/net/wireless/iwlwifi/pcie/rx.c
> +++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
[...]
> @@ -1150,7 +1149,13 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
> * or due to sporadic interrupts thrown from our NIC. */
> if (!inta) {
> IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
> - goto none;
> + /* re-enable interrupts here since we don't have anything to
> + * service. only Re-enable if disabled by irq and no
> + * schedules tasklet. */
The preferred way of formatting the multi-line comments in the networking
code is this:
/* bla
* bla
*/
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCHv2] net: wireless: iwlwifi: remove minor dead code
From: Michal Nazarewicz @ 2013-11-10 20:17 UTC (permalink / raw)
To: Sergei Shtylyov, Johannes Berg, Emmanuel Grumbach,
John W. Linville
Cc: Intel Linux Wireless, linux-wireless, netdev, linux-kernel
In-Reply-To: <527FDB50.1050902@cogentembedded.com>
inta is checked to be zero in a IRQ_NONE branch so afterwards it
cannot be zero as it is never modified.
---
drivers/net/wireless/iwlwifi/pcie/rx.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
On Sun, Nov 10 2013, Sergei Shtylyov wrote:
> The preferred way of formatting the multi-line comments in the
> networking code is this:
>
> /* bla
> * bla
> */
Works for me. I used /* bla \n * bla */ because this is what the rest
of this function was using.
diff --git a/drivers/net/wireless/iwlwifi/pcie/rx.c b/drivers/net/wireless/iwlwifi/pcie/rx.c
index 3f237b4..7d0fdc0 100644
--- a/drivers/net/wireless/iwlwifi/pcie/rx.c
+++ b/drivers/net/wireless/iwlwifi/pcie/rx.c
@@ -1121,7 +1121,6 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
struct iwl_trans *trans = data;
struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
u32 inta, inta_mask;
- irqreturn_t ret = IRQ_NONE;
lockdep_assert_held(&trans_pcie->irq_lock);
@@ -1150,7 +1149,14 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
* or due to sporadic interrupts thrown from our NIC. */
if (!inta) {
IWL_DEBUG_ISR(trans, "Ignore interrupt, inta == 0\n");
- goto none;
+ /* re-enable interrupts here since we don't have anything to
+ * service. only Re-enable if disabled by irq and no
+ * schedules tasklet.
+ */
+ if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
+ !trans_pcie->inta)
+ iwl_enable_interrupts(trans);
+ return IRQ_NONE;
}
if ((inta == 0xFFFFFFFF) || ((inta & 0xFFFFFFF0) == 0xa5a5a5a0)) {
@@ -1168,19 +1174,7 @@ static irqreturn_t iwl_pcie_isr(int irq, void *data)
trans_pcie->inta |= inta;
/* the thread will service interrupts and re-enable them */
- if (likely(inta))
- return IRQ_WAKE_THREAD;
-
- ret = IRQ_HANDLED;
-
-none:
- /* re-enable interrupts here since we don't have anything to service. */
- /* only Re-enable if disabled by irq and no schedules tasklet. */
- if (test_bit(STATUS_INT_ENABLED, &trans_pcie->status) &&
- !trans_pcie->inta)
- iwl_enable_interrupts(trans);
-
- return ret;
+ return IRQ_WAKE_THREAD;
}
/* interrupt handler using ict table, with this interrupt driver will
--
1.8.3.2
^ 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