* [PATCH RFC] net: error on trying to add a duplicate ipip tunnel
From: Andreas Henriksson @ 2013-11-13 18:21 UTC (permalink / raw)
To: netdev
After patch:
$ sudo ip tunnel add test1 mode ipip remote 1.2.3.4
$ sudo ip tunnel add test2 mode ipip remote 1.2.3.4
add tunnel "tunl0" failed: File exists
Before the patch, there would be no error and
"test2" (silently) not added.
Originally reported at http://bugs.debian.org/508450
The originally reported problem with sit tunnel seems
to have been resolved since then:
$ sudo ip tun add test3 mode sit
add tunnel "sit0" failed: No buffer space available
The problem still exists for (atleast) ipip tunnels though.
Reported-by: martin f krafft <madduck@debian.org>
Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
net/ipv4/ip_tunnel.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Maybe also sit tunnels should explicitly
use EEXISTS instead of ENOBUFS?
Maybe ipip tunnel code should be refactored
to be similar to sit tunnel addition code?
Which other tunnel types should be checked
for how they behave in similar situations?
Other comments?
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 63a6d6d..1dc4e41 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -732,8 +732,14 @@ int ip_tunnel_ioctl(struct net_device *dev, struct ip_tunnel_parm *p, int cmd)
t = ip_tunnel_find(itn, p, itn->fb_tunnel_dev->type);
- if (!t && (cmd == SIOCADDTUNNEL))
- t = ip_tunnel_create(net, itn, p);
+ if (cmd == SIOCADDTUNNEL) {
+ if (!t) {
+ t = ip_tunnel_create(net, itn, p);
+ } else {
+ err = -EEXIST;
+ break;
+ }
+ }
if (dev != itn->fb_tunnel_dev && cmd == SIOCCHGTUNNEL) {
if (t != NULL) {
--
1.8.4.3
^ permalink raw reply related
* Re: [PATCH v2] mac80211: add assoc beacon timeout logic
From: Johannes Berg @ 2013-11-13 18:30 UTC (permalink / raw)
To: Felipe Contreras
Cc: linux-wireless Mailing List, netdev, John W. Linville,
David S. Miller
In-Reply-To: <CAMP44s1yJMd+9G5yu988GqafRYnAteN-y-WWDe-rJh_qQpSPkQ@mail.gmail.com>
On Mon, 2013-11-11 at 12:06 -0600, Felipe Contreras wrote:
> > The not receiving part is a bug. I think you're probably receiving
> > beacons once associated though?
>
> Nope. Never.
That's odd, firmware bug? But it's still odd since windows works?
> Moreover, if continuing the association without beacons has a legal a
> problem, that problem would exist for drivers that don't have the
> IEEE80211_HW_NEED_DTIM_BEFORE_ASSOC flag, wouldn't it? How exactly
> would trying to associate with need_beacon break the law, but not if
> !need_beacon?
Well, it's actually somewhat unlikely you'd break the law, although
possible, since eventually you'd get disconnected from the AP anyway?
In any case - your argumentation above isn't true because every device
listens for beacons, and if none are received then it should eventually
disconnect. That should be true even with your hack here.
In any case, I'm not really comfortable connecting to an AP that we
never ever receive beacons from. I can try to investigate why this
doesn't happen on windows and what we should be doing though.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: minstrels: spare numerous useless calls to get_random_bytes
From: Johannes Berg @ 2013-11-13 18:34 UTC (permalink / raw)
To: Karl Beldan
Cc: Felix Fietkau, linux-wireless, Hannes Frederic Sowa, netdev,
Karl Beldan
In-Reply-To: <1384336459-5322-1-git-send-email-karl.beldan@gmail.com>
On Wed, 2013-11-13 at 10:54 +0100, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
>
> ATM, only the first array value returned by get_random_bytes is used.
> This change moves the call to get_random_bytes from the nested loop it
> is in to its parent.
> While at it, replace get_random_bytes with prandom_bytes since PRNs are
> way enough for the selection process.
> After this, minstrel_ht reclaims 80 PR-bytes instead of 640 R-bytes.
Applied.
johannes
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Chang @ 2013-11-13 19:06 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Vlad Yasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
dreibh
In-Reply-To: <52833BD3.4040003@redhat.com>
On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
> On 11/13/2013 03:54 AM, Chang wrote:
>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>> Look for the __two__ most recently used path/transport and set to
>>>> active_path
>>>> and retran_path respectively
>
> Please also for the log, elaborate a bit more, explaining what currently
> happens, and what the effects of this bug are, so that later when people
> are looking through the Git log they can easily get what problem you are
> trying to fix; and if possible, add:
>
> Fixes: <12 digits SHA1> ("<commit title>")
>
Yeah, sure, I'll elaborate that more specifically.
I assume the 12-digit SHA1 is the revision number. But may I ask where
and how shall I add the tag "Fixes" tag? The revision number is
generated after "git commit", how can I know that in advance?
Best Regards!
>>>> Signed-off-by: changxiangzhong@gmail.com
>>>> ---
>>>> net/sctp/associola.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>>> index ab67efc..070011a 100644
>>>> --- a/net/sctp/associola.c
>>>> +++ b/net/sctp/associola.c
>>>> @@ -913,11 +913,15 @@ void sctp_assoc_control_transport(struct
>>>> sctp_association *asoc,
>>>> if (!first || t->last_time_heard > first->last_time_heard) {
>>>> second = first;
>>>> first = t;
>>>> + continue;
>>>> }
>>>> if (!second || t->last_time_heard > second->last_time_heard)
>>>> second = t;
>>>
>>> You might as well remove this bit and then you don't need a continue.
>> I don't think we could remove this bit. My understanding of these
>> algorithms are to find the 1st recently used path and the 2nd,
>> assigning to active_path and retran_path respectively. If we remove
>> the looking-for-second block, how are we suppose to find the 2nd?
>> I think we can remove the continue and use else-if in the
>> 2nd-assignment-block.
>>>
>>>> }
>>>>
>>>> + if (!second)
>>>> + second = first;
>>>> +
>>>
>>> This needs to move down 1 more block. Set the second transport
>>> after we
>>> check to see if the primary is back up and we need to go back to
>>> using it.
>>>
>>> -vlad
>>>
>> I agree with this change
>>>> /* RFC 2960 6.4 Multi-Homed SCTP Endpoints
>>>> *
>>>> * By default, an endpoint should always transmit to the
>>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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
* __refrigerator() && saved task->state
From: Oleg Nesterov @ 2013-11-13 19:11 UTC (permalink / raw)
To: Tejun Heo
Cc: Peter Zijlstra, David Rientjes, David Laight, Geert Uytterhoeven,
Ingo Molnar, netdev, linux-kernel
In-Reply-To: <20131113170724.GA17739@redhat.com>
Sorry for noise, but I am totally confused.
Could you please remind why __refrigerator() saves/restores
task->state?
I can see only one reason: set_freezable() kernel threads which
check kthread_should_stop() and do try_to_freeze() by hand.
But does this save/restore actually help?
For example kauditd_thread() looks obviously racy exactly because
try_to_freeze() can return in TASK_INTERRUPTIBLE state after
wake_up(kauditd_wait) was already called, we can miss an event.
At first glance it would be better to simply kill this logic? If
it was called with ->state != 0, the caller is going to schedule()
and it probably executes the wait_event-like code, in this case
it would me more safe to pretend the task got a spurious wakeup?
(as for kauditd_thread() in particular, it looks wrong in any
case, even kthread_should_stop() check doesn't look right, it
needs kthread_freezable_should_stop() afaics).
But I guess I missed something else...
Oleg.
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Daniel Borkmann @ 2013-11-13 19:10 UTC (permalink / raw)
To: Chang
Cc: Vlad Yasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
dreibh
In-Reply-To: <5283CDD1.3090309@gmail.com>
On 11/13/2013 08:06 PM, Chang wrote:
> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>> On 11/13/2013 03:54 AM, Chang wrote:
>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>> Look for the __two__ most recently used path/transport and set to active_path
>>>>> and retran_path respectively
>>
>> Please also for the log, elaborate a bit more, explaining what currently
>> happens, and what the effects of this bug are, so that later when people
>> are looking through the Git log they can easily get what problem you are
>> trying to fix; and if possible, add:
>>
>> Fixes: <12 digits SHA1> ("<commit title>")
>>
> Yeah, sure, I'll elaborate that more specifically.
Thanks !
> I assume the 12-digit SHA1 is the revision number. But may I ask where and how shall I add the tag "Fixes" tag? The revision number is generated after "git commit", how can I know that in advance?
Nope, it's the affected commit id from the current git log that
your patch fixes.
Have a look for example at commit:
98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
^ permalink raw reply
* Re: __refrigerator() && saved task->state
From: Peter Zijlstra @ 2013-11-13 19:14 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tejun Heo, David Rientjes, David Laight, Geert Uytterhoeven,
Ingo Molnar, netdev, linux-kernel
In-Reply-To: <20131113191143.GA24005@redhat.com>
On Wed, Nov 13, 2013 at 08:11:43PM +0100, Oleg Nesterov wrote:
> At first glance it would be better to simply kill this logic? If
> it was called with ->state != 0, the caller is going to schedule()
> and it probably executes the wait_event-like code, in this case
> it would me more safe to pretend the task got a spurious wakeup?
Note that in general the kernel cannot deal with spurious wakeups :/
Most proper locks and wait primitives can, but there's enough open-coded
crap out there that can not.
I tried..
^ permalink raw reply
* Re: __refrigerator() && saved task->state
From: Oleg Nesterov @ 2013-11-13 19:40 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, David Rientjes, David Laight, Geert Uytterhoeven,
Ingo Molnar, netdev, linux-kernel
In-Reply-To: <20131113191449.GF16796@laptop.programming.kicks-ass.net>
On 11/13, Peter Zijlstra wrote:
>
> On Wed, Nov 13, 2013 at 08:11:43PM +0100, Oleg Nesterov wrote:
> > At first glance it would be better to simply kill this logic? If
> > it was called with ->state != 0, the caller is going to schedule()
> > and it probably executes the wait_event-like code, in this case
> > it would me more safe to pretend the task got a spurious wakeup?
>
> Note that in general the kernel cannot deal with spurious wakeups :/
>
> Most proper locks and wait primitives can, but there's enough open-coded
> crap out there that can not.
Oh yes, I understand.
My point is, "restore the old state" in this case looks worse simply
because you miss any wakeup in between which was going to clear that
state. And afaics only kthreads can call __refrigerator() in !RUNNING.
But let me repeat, I am almost sure I missed something else.
Oleg.
^ permalink raw reply
* Fwd: Cross thread shutdown of connected UDP socket, unexpected recvfrom behavior
From: mpb @ 2013-11-13 20:07 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAApbN=Jm=zuDaVYALmGg5z_QfJJ3q13e9tshb7OpAtPQ8Ema1w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
Hi netdev@vger.kernel.org,
Someone on LKML suggested that I forward this issue to the netdev list.
In a nutshell, recvfrom returns ambiguous results, and it is
impossible to distinguish between these two situations:
a) calling recvfrom and receiving a zero length UDP message
b) calling recvfrom and blocking until the socket is shutdown (locally)
More details below.
Thanks!
-mpb
---------- Forwarded message ----------
From: mpb <mpb.mail@gmail.com>
Date: Sat, Nov 9, 2013 at 5:32 PM
Subject: Cross thread shutdown of connected UDP socket, unexpected
recvfrom behavior
To: linux-kernel@vger.kernel.org
Hi LKML,
I have a C/pthreads program with two threads ("main" and "thread").
"thread" calls recvfrom on a connected UDP socket and blocks, waiting
for input. "main" then calls shutdown SHUT_RD on the socket. In
"thread", recvfrom returns, apparently successfully, returning a zero
length string, and setting src_addr to a bogus(?) address family,
port, and source address.
Is the above the correct behavior for recvfrom?
Here is output from my program. "main" sends "hello\0", then "" (the
empty string), then calls shutdown. "thread" receives "hello\0", "",
and then finally receives an empty string that was never sent!
thread recvfrom: Success
rv 6 addrlen 16 fam 2 port 8000 addr 100007f
thread recvfrom: Success
rv 0 addrlen 16 fam 2 port 8000 addr 100007f
main shutdown: Success
rv 0
thread recvfrom: Success
rv 0 addrlen 16 fam 59060 port 44237 addr deaadef0
The source code (2k) for the porgram is attached. I'm running Ubuntu
13.04, kernel 3.8.0-19-generic #30-Ubuntu, on 32-bit i686.
For reference, this June 2000 LKML thread discusses calling close in a
similar situation.
http://www.gossamer-threads.com/lists/linux/kernel/144379
Please CC me if you have questions, otherwise I'll try to watch for
answers in the list archives.
Thanks!
-mpb
[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 1861 bytes --]
#include <netinet/in.h>
#include <pthread.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
struct sockaddr_in addr_0, addr_1, addr_2 ;
int fd0, fd1, rv0, rv1, addrlen;
char buf_0[1024], buf_1[1024];
pthread_t thread;
void* thread_rv;
void* thread_main (void* arg) {
int i; for ( i=0; i<3; i++ ) {
addr_2.sin_family = 0;
addr_2.sin_port = 0;
addr_2.sin_addr.s_addr = 0;
addrlen = sizeof addr_2;
rv1 = recvfrom ( fd1, buf_0, sizeof buf_0, 0,
(struct sockaddr*) &addr_2, &addrlen );
int fam = addr_2.sin_family;
int port = addr_2.sin_port;
int addr = addr_2.sin_addr.s_addr;
printf ("\n");
perror ("thread recvfrom");
printf ( " rv %d addrlen %d fam %d port %d addr %x\n",
rv1, addrlen, fam, ntohs (port), addr );
;;; }
return NULL; }
void main_main () {
send ( fd0, "hello", 6, 0);
send ( fd0, "", 0, 0);
usleep (100000);
rv0 = shutdown ( fd1, SHUT_RD );
printf ("\n");
perror ("main shutdown");
printf (" %d\n", rv0)
;;; }
int main () {
addr_0.sin_family = AF_INET;
addr_0.sin_port = htons (8000);
inet_pton ( AF_INET, "127.0.0.1", &addr_0.sin_addr );
addr_1.sin_family = AF_INET;
addr_1.sin_port = htons (8001);
inet_pton ( AF_INET, "127.0.0.1", &addr_1.sin_addr );
fd0 = socket ( AF_INET, SOCK_DGRAM, 0 );
bind ( fd0, (struct sockaddr*) &addr_0, sizeof addr_0 );
connect ( fd0, (struct sockaddr*) &addr_1, sizeof addr_1 );
fd1 = socket ( AF_INET, SOCK_DGRAM, 0 );
bind ( fd1, (struct sockaddr*) &addr_1, sizeof addr_1 );
connect ( fd1, (struct sockaddr*) &addr_0, sizeof addr_0 );
pthread_create ( &thread, NULL, thread_main, NULL );
main_main ();
pthread_join ( thread, &thread_rv );
return 0; }
^ permalink raw reply
* Re: how to mix bridges and bonding inc. vlans correctly on Kernel > 3.10
From: Stefan Priebe @ 2013-11-13 20:09 UTC (permalink / raw)
To: Vlad Yasevich, Veaceslav Falico; +Cc: Linux Netdev List
In-Reply-To: <5283B500.9040408@gmail.com>
Am 13.11.2013 18:21, schrieb Vlad Yasevich:
> On 11/13/2013 11:21 AM, Veaceslav Falico wrote:
>> On Wed, Nov 13, 2013 at 04:17:33PM +0100, Stefan Priebe - Profihost AG
>> wrote:
>>> Am 13.11.2013 16:05, schrieb Vlad Yasevich:
>>>> On 11/13/2013 09:20 AM, Stefan Priebe - Profihost AG wrote:
>>>>> Hi Falico,
>>>>> Am 13.11.2013 15:12, schrieb Veaceslav Falico:
>>>>>> On Wed, Nov 13, 2013 at 02:58:40PM +0100, Stefan Priebe -
>>>>>> Profihost AG
>>>>>> wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> while my vlans, bridging and bonding stuff was working until 3.9 i
>>>>>>> never
>>>>>>> thought about how it is right. So maybe i was always wrong.
>>>>>>>
>>>>>>> I've this:
>>>>>>>
>>>>>>> eth2
>>>>>>> \
>>>>>>> -- bond1 -- vmbr1
>>>>>>> /
>>>>>>> eth3
>>>>>>>
>>>>>>> This works fine and as expected now i want to have a vlan using the
>>>>>>> bonding and using a bridge.
>>>>>>>
>>>>>>> I the past i had this:
>>>>>>> eth2
>>>>>>> \
>>>>>>> -- bond1 -- vmbr1
>>>>>>> / \
>>>>>>> eth3 \ vmbr1.3000
>>>>>>> \ ---- tap114i1
>>>>>>>
>>>>>>> This was working fine until 3.9.X since 3.10. Right now using 3.10 i
>>>>>>> need to put eth2 and eth3 into promisc mode to get it working ;-(
>>>>>>> this
>>>>>>> is bad!
>>>>>>
>>>>>> As a guess - do you use arp monitoring for bonding? Try using
>>>>>> miimon -
>>>>>> there were some issues with it in 3.10, which were fixed by some huge
>>>>>> patchsets that will never hit 3.10 stable.
>>>>>> Also, the bonding configuration would be welcome.
>>>>>
>>>>> Debian Bonding konfiguration looks like this:
>>>>> auto bond1
>>>>> iface bond1 inet manual
>>>>> slaves eth2 eth3
>>>>> bond-mode 802.3ad
>>>>> bond_miimon 100
>>>>> bond_updelay 200
>>>>> bond_downdelay 0
>>>>>
>>>>> This should be miimon using lacp and not arp isn't it?
>>>>> Anything more needed?
>>>>>
>>>>
>>>> Hmm.. With 802.3ad mode, when the bond is a port on the bridge, the
>>>> bond should place all of its ports into promiscuous mode. Do you see
>>>> the the kernel messages that say that?
>>>
>>> No it does not - i only see:
>>> # dmesg -c|egrep "promiscuous|forward"
>>> [ 5.445161] device bond0 entered promiscuous mode
>>> [ 7.670701] device bond1 entered promiscuous mode
>>> [ 7.845472] vmbr0: port 1(bond0) entered forwarding state
>>> [ 7.845474] vmbr0: port 1(bond0) entered forwarding state
>>> [ 8.269769] vmbr1: port 1(bond1) entered forwarding state
>>> [ 8.269771] vmbr1: port 1(bond1) entered forwarding state
>>>
>>> Now adding variant 1:
>>> # dmesg -c|egrep "promiscuous|forward"
>>> [ 85.919382] device tap113i0 entered promiscuous mode
>>> [ 85.965018] vmbr0: port 2(tap113i0) entered forwarding state
>>> [ 85.965023] vmbr0: port 2(tap113i0) entered forwarding state
>>> [ 86.263292] device tap113i1 entered promiscuous mode
>>> [ 86.314151] device vmbr1.3000 entered promiscuous mode
>>> [ 86.314153] device vmbr1 entered promiscuous mode
>>> [ 86.314192] vmbr1v3000: port 1(vmbr1.3000) entered forwarding state
>>> [ 86.314196] vmbr1v3000: port 1(vmbr1.3000) entered forwarding state
>>> [ 86.318116] vmbr1v3000: port 2(tap113i1) entered forwarding state
>>> [ 86.318120] vmbr1v3000: port 2(tap113i1) entered forwarding state
>>> [ 101.382129] vmbr1v3000: port 1(vmbr1.3000) entered forwarding state
>>>
>>> Now it looks like this:
>>> # ip a l|grep PROMISC
>>> 13: tap113i0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc
>>> htb master vmbr0 state UNKNOWN qlen 500
>>> 14: tap113i1: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc
>>> htb master vmbr1v3000 state UNKNOWN qlen 500
>>
>> eth* should get into forwarding mode cause bond0 is a port of the bridge
>> and should propagate its state towards its slaves. Something is wrong
>> here.
>>
>> Maybe we're looking at the wrong direction - and the promisc for the
>> ethernet drivers got broken?
>
> I was able to duplicate Stefans results only when I turn off the link to
> the underlying devices when building the bridge. When the link is off,
> the rx_flags do not propagate down to the lower level devices.
>
> What about something like this (completely untested)
>
> diff --git a/drivers/net/bonding/bond_main.c
> b/drivers/net/bonding/bond_main.c
> index 4dd5ee2..3051744 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2863,6 +2863,17 @@ static int bond_slave_netdev_event(unsigned long
> event,
> bond_release(bond_dev, slave_dev);
> break;
> case NETDEV_UP:
> + /* If the bond was set to primisc, but slave has not due to
> + * slave being down when the command was issued, sync the
> + * state when the slave comes up.
> + */
> + if (bond_dev->flags & IFF_PROMISC &&
> !slave_dev->promiscuity) {
> + if (!USES_PRIMARY(bond))
> + dev_set_promiscuity(slave_dev, 1);
> + else if (slave == bond->curr_active_slave)
> + dev_set_promiscuity(slave_dev, 1);
> + }
> +
> case NETDEV_CHANGE:
> old_speed = slave->speed;
> old_duplex = slave->duplex;
>
> -vlad
>>
I tried that one but it still looks like this:
[ 173.266915] device tap113i1 entered promiscuous mode
[ 173.305617] 8021q: adding VLAN 3000 to HW filter on device bond1.3000
[ 173.315881] device bond1.3000 entered promiscuous mode
[ 173.315926] vmbr1v3000: port 1(bond1.3000) entered forwarding state
[ 173.315929] vmbr1v3000: port 1(bond1.3000) entered forwarding state
[ 173.319844] vmbr1v3000: port 2(tap113i1) entered forwarding state
[ 173.319847] vmbr1v3000: port 2(tap113i1) entered forwarding state
[ 188.344076] vmbr1v3000: port 1(bond1.3000) entered forwarding state
# ip a l|grep PROMISC
13: tap113i0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc
htb master vmbr0 state UNKNOWN qlen 500
14: tap113i1: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc
htb master vmbr1v3000 state UNKNOWN qlen 500
>> What ethernet cards/driver do you use for eth*?
>>
>>>
>>> Greets,
>>> Stefan
>>>
>>>
>>> Main question is - is this one correct:
>>
>> Both are correct. Here's my setup (sorry for stretching):
>>
>> +---------------+ +------------+
>> +-------------+ +---------+ +------+
>> | bond1 | | | | bridge0
>> | | | | |
>> | 192.168.2.1 | master | bridge0.15 | neighbour | 192.168.3.1 |
>> master | bond0 | master | eth2 |
>> | | --------> | | ------------ | 192.168.4.1 |
>> --------> | | --------> | |
>> +---------------+ +------------+
>> +-------------+ +---------+ +------+
>>
>> |
>>
>> | master
>>
>> v
>> +---------------+
>> +---------+
>> | dummy0
>> | |
>> eth0 |
>> +---------------+
>> +---------+
>>
>> (disregard that dummy0).
>>
>> All 192.168.X.1 ips are pingable (via the correct vlans) on both
>> net-next and stable 3.10.19.
>>
>>>>>>> eth2
>>>>>>> \
>>>>>>> -- bond1 -- vmbr1
>>>>>>> / \
>>>>>>> eth3 \ vmbr1.3000
>>>>>>> \ ---- tap114i1
>>>
>>> <= does not work at all
>>>
>>> or this one?:
>>>>>>> eth2
>>>>>>> \
>>>>>>> -- bond1 -- vmbr1
>>>>>>> / \
>>>>>>> eth3 ----- bond1.3000 --- vmbr1v3000
>>>>>>> \ ---- tap114i1
>>>
>>> <= works if i manually put eth2 and eth3 into promiscous mode.
>>>
>>>> -vlad
>>>>
>>>>> One thing i forgot the one with vmbr1.3000 does not work at all eben
>>>>> not
>>>>> with promisc mode. The one below works fine if i set eth2 and eth3
>>>>> into
>>>>> promisc mode.
>>>>>
>>>>> Stefan
>>>>>
>>>>>>> I also tried this one without success:
>>>>>>> eth2
>>>>>>> \
>>>>>>> -- bond1 -- vmbr1
>>>>>>> / \
>>>>>>> eth3 ----- bond1.3000 --- vmbr1v3000
>>>>>>> \ ---- tap114i1
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Greets,
>>>>>>> Stefan
>>>>>>> --
>>>>>>> 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
>>>>> --
>>>>> 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/7990: Make lance_private.name const
From: David Miller @ 2013-11-13 20:13 UTC (permalink / raw)
To: geert; +Cc: sergei.shtylyov, netdev, linux-m68k
In-Reply-To: <CAMuHMdXyqOATvE8e=3Ed17bwiDRE8w=B=xKuYwd5qorxqA0BJg@mail.gmail.com>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 13 Nov 2013 09:54:18 +0100
> On Wed, Nov 13, 2013 at 12:16 AM, David Miller <davem@davemloft.net> wrote:
>>>> And remember about checkpatch.pl which was hardly content with the patch.
>>>
>>> Only because checkpatch looks at the _new_ lines, and doesn't compare the
>>> styles of the old and new lines.
>>
>> You really should fix the line you are changing to use tabs, please respin
>> with this in mind, thanks.
>
> As that would add more to the TAB/space mess^H^H^Hix, I prepended a
> patch to fix all whitespace errors.
I implicitly asked you not to do this, now things are more difficult
and your original change will take longer to integrate.
Becuase now it isn't a patch set I can easily just apply quickly in
the current merge window, and it's therefore a series you'll have to
resubmit later when the merge window closes and the net-next tree
opens up again.
Sorry.
^ permalink raw reply
* Re: [PATCH RFC] netlink: a flag requesting header w/o data in error ACK
From: David Miller @ 2013-11-13 20:31 UTC (permalink / raw)
To: jbenc; +Cc: netdev, tgraf, jhs, pablo
In-Reply-To: <231e025015eee115182368b34333b38579e3112d.1384357520.git.jbenc@redhat.com>
From: Jiri Benc <jbenc@redhat.com>
Date: Wed, 13 Nov 2013 16:46:09 +0100
> Currently, ACK in case of error contains a full copy of the originating
> message, although many applications do not need it. This can cause lost ACKs
> with large netlink messages, especially after commit c05cdb1b864f ("netlink:
> allow large data transfers from user-space").
>
> Introduce a new message flag, NLM_F_SHORT_NACK, which requests only message
> header to be included in ACK regardless of the error code. This flag has no
> effect if NLM_F_ACK is not set.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
> I'm not entirely happy with taking a bit in netlink message header for this.
> Alternative approach would be to make this a socket option (as suggested by
> David) but that would mean a socket lookup in netlink_ack for each and every
> ACK, which doesn't sound nice, either. If this is preferred, I'll rework the
> patch.
It's not really all that difficult, you can either attack the sender socket
to in_skb->sk, or pass a new boolean around in the various code paths that
lead to the ACK.
^ permalink raw reply
* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: David Miller @ 2013-11-13 20:43 UTC (permalink / raw)
To: jbenc; +Cc: pablo, jhs, tgraf, netdev
In-Reply-To: <20131113122504.0ddcbf89@griffin>
From: Jiri Benc <jbenc@redhat.com>
Date: Wed, 13 Nov 2013 12:25:04 +0100
> I completely agree with this, sorry for not being clear. I just
> understood from the thread that the way to go is to do both, in order
> to not generate too large ACKs for the _new_ code (i.e. for the
> messages that were not plausible before "netlink: allow large data
> transfers from user-space"). I don't know what the "too large" should
> be, though, hence the question.
>
> But then, if we don't do any capping, the only outcome of a failed
> allocation is the ACK won't be sent and it's clearly stated that
> netlink does not provide reliability. Works for me.
Of course, we should meanwhile add the large SKB handling to the
netlink ACK code.
Therefore, please resubmit your original patch, but modify it as
I asked such that only the ACK code path gets the new large SKB
call.
Thanks.
^ permalink raw reply
* oops in tcp_get_metrics, followed by lockup.
From: Dave Jones @ 2013-11-13 20:45 UTC (permalink / raw)
To: netdev
My fuzzer just hit this on v3.12-7033-g42a2d923cc34
Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in: fuse hidp tun snd_seq_dummy bnep nfnetlink rfcomm ipt_ULOG can_bcm nfc caif_socket caif af_802154 phonet af_rxrpc bluetooth rfkill can_raw can llc2 pppoe pppox ppp
_generic slhc irda crc_ccitt rds scsi_transport_iscsi af_key rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 xfs libcrc32c coretemp hwmon x86_pkg_temp_thermal kvm_intel kvm crct10dif_p
clmul crc32c_intel ghash_clmulni_intel usb_debug snd_hda_codec_realtek snd_hda_codec_hdmi microcode pcspkr snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_page_alloc snd_ti
mer e1000e snd ptp shpchp soundcore pps_core serio_raw
CPU: 1 PID: 16002 Comm: trinity-child1 Not tainted 3.12.0+ #2
task: ffff88023cd75580 ti: ffff88009ee26000 task.ti: ffff88009ee26000
RIP: 0010:[<ffffffff81658dd2>] [<ffffffff81658dd2>] tcp_get_metrics+0x62/0x420
RSP: 0018:ffff880244a03d28 EFLAGS: 00010246
RAX: 0000000000000002 RBX: ffff88009c77a4c0 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88009c77a4c0
RBP: ffff880244a03d78 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 00000000000010ac R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff880244a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000018 CR3: 0000000001c0b000 CR4: 00000000001407e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Stack:
000000018165a6b5 ffffffff000010ac 0000000000000246 0000000044a00002
ffffffff81c480a0 ffff88009c77a4c0 0000000000000000 0000000000000000
0000000000000001 0000000000000000 ffff880244a03db8 ffffffff8165a740
Call Trace:
<IRQ>
[<ffffffff8165a740>] tcp_fastopen_cache_set+0x90/0x280
[<ffffffff8165a6b5>] ? tcp_fastopen_cache_set+0x5/0x280
[<ffffffff8164f3a7>] tcp_retransmit_timer+0x1d7/0x930
[<ffffffff8164fcb0>] ? tcp_write_timer_handler+0x1b0/0x1b0
[<ffffffff8164fba0>] tcp_write_timer_handler+0xa0/0x1b0
[<ffffffff8164fd2c>] tcp_write_timer+0x7c/0x80
[<ffffffff81063c1a>] call_timer_fn+0x8a/0x340
[<ffffffff81063b95>] ? call_timer_fn+0x5/0x340
[<ffffffff8164fcb0>] ? tcp_write_timer_handler+0x1b0/0x1b0
[<ffffffff81064114>] run_timer_softirq+0x244/0x3a0
[<ffffffff8105aa9c>] __do_softirq+0xfc/0x490
[<ffffffff8105b28d>] irq_exit+0x13d/0x160
[<ffffffff8172fe25>] smp_apic_timer_interrupt+0x45/0x60
[<ffffffff8172eaaf>] apic_timer_interrupt+0x6f/0x80
<EOI>
[<ffffffff810d559d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff811560af>] ? free_hot_cold_page+0xff/0x180
[<ffffffff81156176>] free_hot_cold_page_list+0x46/0x160
[<ffffffff8115c21e>] release_pages+0x8e/0x1f0
[<ffffffff8118c135>] free_pages_and_swap_cache+0x95/0xb0
[<ffffffff81175acc>] tlb_flush_mmu.part.73+0x4c/0x90
[<ffffffff81176115>] tlb_finish_mmu+0x55/0x60
[<ffffffff81180d84>] exit_mmap+0xf4/0x170
[<ffffffff8105108b>] mmput+0x6b/0x100
[<ffffffff810559e8>] do_exit+0x278/0xcb0
[<ffffffff817250e1>] ? _raw_spin_unlock+0x31/0x50
[<ffffffff810d53c6>] ? trace_hardirqs_on_caller+0x16/0x1e0
[<ffffffff810d559d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff810577ec>] do_group_exit+0x4c/0xc0
[<ffffffff81057874>] SyS_exit_group+0x14/0x20
[<ffffffff8172e064>] tracesys+0xdd/0xe2
Code: 0a 0f 85 c2 01 00 00 48 8b 47 38 48 8b 57 40 48 89 44 24 08 48 8b 47 40 48 89 54 24 10 48 33 47 38 49 89 c6 49 c1 ee 20 41 31 c6 <49> 8b 45 18 b9 20 00 00 00 45 69 f6 01 00 37 9e 48 8b 80 d8 04
RIP [<ffffffff81658dd2>] tcp_get_metrics+0x62/0x420
RSP <ffff880244a03d28>
CR2: 0000000000000018
---[ end trace c25bf4de9744120a ]---
The disassembly looks like it happened here :-
static inline u32 ipv6_addr_hash(const struct in6_addr *a)
{
#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
const unsigned long *ul = (const unsigned long *)a;
unsigned long x = ul[0] ^ ul[1];
10db: 48 8b 47 40 mov 0x40(%rdi),%rax
10df: 48 89 54 24 10 mov %rdx,0x10(%rsp)
10e4: 48 33 47 38 xor 0x38(%rdi),%rax
return (u32)(x ^ (x >> 32));
10e8: 49 89 c6 mov %rax,%r14
10eb: 49 c1 ee 20 shr $0x20,%r14
10ef: 41 31 c6 xor %eax,%r14d
10f2: 49 8b 45 18 mov 0x18(%r13),%rax <<<< Faulting instruction.
10f6: b9 20 00 00 00 mov $0x20,%ecx
}
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Chang @ 2013-11-13 20:48 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Vlad Yasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
dreibh
In-Reply-To: <5283CEC3.4050507@redhat.com>
On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
> On 11/13/2013 08:06 PM, Chang wrote:
>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>> Look for the __two__ most recently used path/transport and set to
>>>>>> active_path
>>>>>> and retran_path respectively
>>>
>>> Please also for the log, elaborate a bit more, explaining what
>>> currently
>>> happens, and what the effects of this bug are, so that later when
>>> people
>>> are looking through the Git log they can easily get what problem you
>>> are
>>> trying to fix; and if possible, add:
>>>
>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>
>> Yeah, sure, I'll elaborate that more specifically.
>
> Thanks !
>
>> I assume the 12-digit SHA1 is the revision number. But may I ask
>> where and how shall I add the tag "Fixes" tag? The revision number is
>> generated after "git commit", how can I know that in advance?
>
> Nope, it's the affected commit id from the current git log that
> your patch fixes.
>
> Have a look for example at commit:
>
> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
Thank you for your quick response. I'm quite green on kernel programming
and git. So here's one question:
To find the the revision that **caused** the bug, I could use gitk to
trace the changing of the file(s) history. Is that correct?
^ permalink raw reply
* [PATCH 1/2] net: phylib: add adjust_state callback to phy device
From: Daniel Mack @ 2013-11-13 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, marek.belisko, ujhelyi.m, Daniel Mack
Allow phy drivers to take action when the core does its link adjustment.
No change for drivers that do not implement this callback.
Signed-off-by: Daniel Mack <zonque@gmail.com>
---
drivers/net/phy/phy.c | 3 +++
include/linux/phy.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 36c6994..240e33f 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -748,6 +748,9 @@ void phy_state_machine(struct work_struct *work)
if (phydev->adjust_state)
phydev->adjust_state(phydev->attached_dev);
+ if (phydev->drv->adjust_state)
+ phydev->drv->adjust_state(phydev);
+
switch(phydev->state) {
case PHY_DOWN:
case PHY_STARTING:
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 64ab823..85826eb 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -467,6 +467,8 @@ struct phy_driver {
/* See set_wol, but for checking whether Wake on LAN is enabled. */
void (*get_wol)(struct phy_device *dev, struct ethtool_wolinfo *wol);
+ void (*adjust_state)(struct phy_device *dev);
+
struct device_driver driver;
};
#define to_phy_driver(d) container_of(d, struct phy_driver, driver)
--
1.8.4.2
^ permalink raw reply related
* [PATCH 2/2] net: phy: at803x: soft-reset PHY when link goes down
From: Daniel Mack @ 2013-11-13 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, marek.belisko, ujhelyi.m, Daniel Mack
In-Reply-To: <1384376870-7810-1-git-send-email-zonque@gmail.com>
When the link goes down and up again quickly and multiple times in a
row, the AT803x PHY gets stuck and won't recover unless it is soft-reset
via the BMCR register.
Unfortunately, there is no way to detect this state, as all registers
contain perfectly sane values in such cases. Hence, the only option is
to always soft-reset the PHY when the link goes down.
Reset logic is based on a patch from Marek Belisko.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-and-tested-by: Marek Belisko <marek.belisko@gmail.com>
---
drivers/net/phy/at803x.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index bc71947..303c5ae 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -32,10 +32,56 @@
#define AT803X_DEBUG_SYSTEM_MODE_CTRL 0x05
#define AT803X_DEBUG_RGMII_TX_CLK_DLY BIT(8)
+#define AT803X_BMCR_SOFTRESET BIT(15)
+
MODULE_DESCRIPTION("Atheros 803x PHY driver");
MODULE_AUTHOR("Matus Ujhelyi");
MODULE_LICENSE("GPL");
+struct at803x_priv {
+ bool phy_reset;
+};
+
+static void at803x_adjust_state(struct phy_device *phydev)
+{
+ struct at803x_priv *priv = phydev->priv;
+
+ if (phydev->state == PHY_NOLINK) {
+ unsigned long timeout;
+ unsigned int val;
+
+ if (priv->phy_reset)
+ return;
+
+ val = phy_read(phydev, MII_BMCR);
+ val |= AT803X_BMCR_SOFTRESET;
+ phy_write(phydev, MII_BMCR, val);
+
+ timeout = jiffies + HZ/10;
+ do {
+ cpu_relax();
+ } while ((phy_read(phydev, MII_BMCR) & AT803X_BMCR_SOFTRESET) &&
+ time_after(timeout, jiffies));
+
+ WARN(phy_read(phydev, MII_BMCR) & AT803X_BMCR_SOFTRESET,
+ "failed to soft-reset phy\n");
+
+ priv->phy_reset = true;
+ } else {
+ priv->phy_reset = false;
+ }
+}
+
+static int at803x_probe(struct phy_device *phydev)
+{
+ phydev->priv = devm_kzalloc(&phydev->dev, sizeof(struct at803x_priv),
+ GFP_KERNEL);
+ if (!phydev->priv)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int at803x_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
@@ -197,6 +243,7 @@ static struct phy_driver at803x_driver[] = {
.phy_id = 0x004dd072,
.name = "Atheros 8035 ethernet",
.phy_id_mask = 0xffffffef,
+ .probe = at803x_probe,
.config_init = at803x_config_init,
.set_wol = at803x_set_wol,
.get_wol = at803x_get_wol,
@@ -206,6 +253,7 @@ static struct phy_driver at803x_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
+ .adjust_state = at803x_adjust_state,
.driver = {
.owner = THIS_MODULE,
},
@@ -214,6 +262,7 @@ static struct phy_driver at803x_driver[] = {
.phy_id = 0x004dd076,
.name = "Atheros 8030 ethernet",
.phy_id_mask = 0xffffffef,
+ .probe = at803x_probe,
.config_init = at803x_config_init,
.set_wol = at803x_set_wol,
.get_wol = at803x_get_wol,
@@ -223,6 +272,7 @@ static struct phy_driver at803x_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
+ .adjust_state = at803x_adjust_state,
.driver = {
.owner = THIS_MODULE,
},
@@ -231,6 +281,7 @@ static struct phy_driver at803x_driver[] = {
.phy_id = 0x004dd074,
.name = "Atheros 8031 ethernet",
.phy_id_mask = 0xffffffef,
+ .probe = at803x_probe,
.config_init = at803x_config_init,
.set_wol = at803x_set_wol,
.get_wol = at803x_get_wol,
@@ -240,6 +291,7 @@ static struct phy_driver at803x_driver[] = {
.flags = PHY_HAS_INTERRUPT,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
+ .adjust_state = at803x_adjust_state,
.driver = {
.owner = THIS_MODULE,
},
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH v2] tcp: tsq: restore minimal amount of queueing
From: Arnaud Ebalard @ 2013-11-13 21:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Sujith Manoharan, Cong Wang, netdev, Felix Fietkau
In-Reply-To: <1384353174.28458.110.camel@edumazet-glaptop2.roam.corp.google.com>
Hi Eric,
Eric Dumazet <eric.dumazet@gmail.com> writes:
> From: Eric Dumazet <edumazet@google.com>
>
> After commit c9eeec26e32e ("tcp: TSQ can use a dynamic limit"), several
> users reported throughput regressions, notably on mvneta and wifi
> adapters.
>
> 802.11 AMPDU requires a fair amount of queueing to be effective.
>
> This patch partially reverts the change done in tcp_write_xmit()
> so that the minimal amount is sysctl_tcp_limit_output_bytes.
I just tested the fix on current linux tree with the same setup
on which I observed the regression: it also fixes it on my side
on my RN102. Thanks for the quick fix, Eric.
Cheers,
a+
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Chang @ 2013-11-13 21:23 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Vlad Yasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
dreibh
In-Reply-To: <5283E5A8.3020809@gmail.com>
On 11/13/2013 09:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set
>>>>>>> to active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when
>>>> people
>>>> are looking through the Git log they can easily get what problem
>>>> you are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask
>>> where and how shall I add the tag "Fixes" tag? The revision number
>>> is generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel
> programming and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to
> trace the changing of the file(s) history. Is that correct?
>
Let's say the following condition
A - the initial revision
B - something was wrong that introduced a bug
C - latest revision
MyFix - fixing the bug caused by B. Shall I specify B or C in the
"fixes:" tag?
revision introduces a bug
|
A -> B -> C ->MyFix
|
latest revision
^ permalink raw reply
* Re: [PATCH v2 1/1] net:fec: fix WARNING caused by lack of calls to dma_mapping_error()
From: Ben Hutchings @ 2013-11-13 21:27 UTC (permalink / raw)
To: Fugang Duan; +Cc: davem, netdev, b20596
In-Reply-To: <1384311846-4485-1-git-send-email-B38611@freescale.com>
On Wed, 2013-11-13 at 11:04 +0800, Fugang Duan wrote:
[...]
> =============================================
> V2: add net_ratelimit() to limit map err message.
> use dma_sync_single_for_cpu() instead of dma_map_single().
> fix the issue that pass DMA addresses to __va() to get virture address.
> V1: initial send
> =============================================
>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
[...]
No that's not how it works. If I were to thoroughly review this (but I
haven't, and I don't have time to) then I might say 'Reviewed-by: Ben
Hutchings' which you could then add to the patch if re-posting it.
Signed-off-by means something different and you must never add it on
behalf of someone else.
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
* Re: [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr()
From: Julian Anastasov @ 2013-11-13 21:27 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
In-Reply-To: <1384356209-4193-1-git-send-email-nicolas.dichtel@6wind.com>
Hello,
On Wed, 13 Nov 2013, Nicolas Dichtel wrote:
> Help of this function says: "in_dev: only on this interface, 0=any interface",
> but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
> correct namespace."), the code supposes that it will never be NULL. This
> function is never called with in_dev == NULL, but it's exported and may be used
> by an external module.
>
> Because this patch restore the ability to call inet_confirm_addr() with in_dev
> == NULL, I partially revert the above commit, as suggested by Julian.
>
> CC: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
This patch looks ok to me, thanks!
Reviewed-by: Julian Anastasov <ja@ssi.bg>
Still, I think this is a net-next material
and you have to wait some days... The net tree that
you are using is missing the changes that remove
'extern', so I think you have to port it to net-next
tree to avoid the conflict in include/linux/inetdevice.h.
> ---
>
> This bug was spotted by code review.
>
> v2: fix change in bond_confirm_addr()
> partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
> correct namespace.")
> fix API desc of inet_confirm_addr()
>
> drivers/net/bonding/bonding.h | 4 ++--
> include/linux/inetdevice.h | 3 ++-
> net/ipv4/arp.c | 4 +++-
> net/ipv4/devinet.c | 9 ++++-----
> 4 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 03cf3fd14490..0ad3f4bd99c0 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -414,8 +414,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3
> in_dev = __in_dev_get_rcu(dev);
>
> if (in_dev)
> - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
> -
> + addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local,
> + RT_SCOPE_HOST);
> rcu_read_unlock();
> return addr;
> }
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 79640e015a86..5870f7060917 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
> extern void devinet_init(void);
> extern struct in_device *inetdev_by_index(struct net *, int);
> extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
> -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
> + __be32 local, int scope);
> extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
>
> static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7808093cede6..46c7b4483bcf 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -379,6 +379,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
>
> static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> {
> + struct net *net = dev_net(in_dev->dev);
> int scope;
>
> switch (IN_DEV_ARP_IGNORE(in_dev)) {
> @@ -397,6 +398,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> case 3: /* Do not reply for scope host addresses */
> sip = 0;
> scope = RT_SCOPE_LINK;
> + in_dev = NULL;
> break;
> case 4: /* Reserved */
> case 5:
> @@ -408,7 +410,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> default:
> return 0;
> }
> - return !inet_confirm_addr(in_dev, sip, tip, scope);
> + return !inet_confirm_addr(net, in_dev, sip, tip, scope);
> }
>
> static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a1b5bcbd04ae..19426e4b232c 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1236,22 +1236,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
>
> /*
> * Confirm that local IP address exists using wildcards:
> - * - in_dev: only on this interface, 0=any interface
> + * - net: netns to check, cannot be NULL
> + * - in_dev: only on this interface, NULL=any interface
> * - dst: only in the same subnet as dst, 0=any dst
> * - local: address, 0=autoselect the local address
> * - scope: maximum allowed scope value for the local address
> */
> -__be32 inet_confirm_addr(struct in_device *in_dev,
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
> __be32 dst, __be32 local, int scope)
> {
> __be32 addr = 0;
> struct net_device *dev;
> - struct net *net;
>
> - if (scope != RT_SCOPE_LINK)
> + if (in_dev != NULL)
> return confirm_addr_indev(in_dev, dst, local, scope);
>
> - net = dev_net(in_dev->dev);
> rcu_read_lock();
> for_each_netdev_rcu(net, dev) {
> in_dev = __in_dev_get_rcu(dev);
> --
> 1.8.4.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Daniel Borkmann @ 2013-11-13 21:31 UTC (permalink / raw)
To: Chang
Cc: Vlad Yasevich, nhorman, davem, linux-sctp, netdev, linux-kernel,
dreibh
In-Reply-To: <5283EDCF.20605@gmail.com>
On 11/13/2013 10:23 PM, Chang wrote:
[...]
> Let's say the following condition
> A - the initial revision
> B - something was wrong that introduced a bug
> C - latest revision
> MyFix - fixing the bug caused by B. Shall I specify B or C in the "fixes:" tag?
>
>
> revision introduces a bug
> |
> A -> B -> C ->MyFix
> |
> latest revision
You would specify B in the "Fixes:" tag, as B introduced this bug first.
In case there is no suitable git-blame information available in the current
tree (so that bug is pre Linux-2.6.12-rc2 which I believe on a quick look
that this seems the case), you can omit that. Then, just elaborate on the
commit message, and resubmit with feedback from Vlad included, too.
Thanks !
^ permalink raw reply
* Re: [PATCH 1/1] net: sctp: bug fixing when sctp path recovers
From: Vlad Yasevich @ 2013-11-13 21:39 UTC (permalink / raw)
To: Chang, Daniel Borkmann
Cc: nhorman, davem, linux-sctp, netdev, linux-kernel, dreibh
In-Reply-To: <5283E5A8.3020809@gmail.com>
On 11/13/2013 03:48 PM, Chang wrote:
>
> On 11/13/2013 08:10 PM, Daniel Borkmann wrote:
>> On 11/13/2013 08:06 PM, Chang wrote:
>>> On 11/13/2013 09:44 AM, Daniel Borkmann wrote:
>>>> On 11/13/2013 03:54 AM, Chang wrote:
>>>>> On 11/13/2013 03:37 AM, Vlad Yasevich wrote:
>>>>>> On 11/12/2013 08:34 PM, Chang Xiangzhong wrote:
>>>>>>> Look for the __two__ most recently used path/transport and set to
>>>>>>> active_path
>>>>>>> and retran_path respectively
>>>>
>>>> Please also for the log, elaborate a bit more, explaining what
>>>> currently
>>>> happens, and what the effects of this bug are, so that later when
>>>> people
>>>> are looking through the Git log they can easily get what problem you
>>>> are
>>>> trying to fix; and if possible, add:
>>>>
>>>> Fixes: <12 digits SHA1> ("<commit title>")
>>>>
>>> Yeah, sure, I'll elaborate that more specifically.
>>
>> Thanks !
>>
>>> I assume the 12-digit SHA1 is the revision number. But may I ask
>>> where and how shall I add the tag "Fixes" tag? The revision number is
>>> generated after "git commit", how can I know that in advance?
>>
>> Nope, it's the affected commit id from the current git log that
>> your patch fixes.
>>
>> Have a look for example at commit:
>>
>> 98bbc06aabac5a2 ("net: x86: bpf: don't forget to free sk_filter (v2)")
> Thank you for your quick response. I'm quite green on kernel programming
> and git. So here's one question:
> To find the the revision that **caused** the bug, I could use gitk to
> trace the changing of the file(s) history. Is that correct?
>
A lot easier is to run 'git blame <filename>" and find the line you
are fixing. The number that line starts with is the short commit id.
In this particular case, don't worry about it. This is a day 1 bug
-vlad
^ permalink raw reply
* [PATCH] net: sctp: bug-fixing: retran_path not set properly after transports recovering (v2)
From: Chang Xiangzhong @ 2013-11-13 21:54 UTC (permalink / raw)
To: vyasevich; +Cc: nhorman, davem, linux-sctp, netdev, Chang Xiangzhong
When a transport recovers due to the new coming sack, SCTP should
iterate all of its transport_list to locate the __two__ most recently used
transport and set to active_path and retran_path respectively. The exising
code does not find the two properly - In case of the following list:
[most-recent] -> [2nd-most-recent] -> ...
Both active_path and retran_path would be set to the 1st element.
The bug happens when:
1) multi-homing
2) failure/partial_failure transport recovers
Both active_path and retran_path would be set to the same most-recent one, in
other words, retran_path would not take its role - an end user might not even
notice this issue.
Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
---
net/sctp/associola.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index ab67efc..a7fc856 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -913,11 +913,12 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
if (!first || t->last_time_heard > first->last_time_heard) {
second = first;
first = t;
- }
- if (!second || t->last_time_heard > second->last_time_heard)
+ } else if (!second ||
+ t->last_time_heard > second->last_time_heard)
second = t;
}
+
/* RFC 2960 6.4 Multi-Homed SCTP Endpoints
*
* By default, an endpoint should always transmit to the
@@ -935,6 +936,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
first = asoc->peer.primary_path;
}
+ if (!second)
+ second = first;
/* If we failed to find a usable transport, just camp on the
* primary, even if it is inactive.
*/
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] tcp: tsq: restore minimal amount of queueing
From: Holger Hoffstaette @ 2013-11-13 21:59 UTC (permalink / raw)
To: netdev
In-Reply-To: <8738n0ngq3.fsf@natisbad.org>
On Wed, 13 Nov 2013 22:18:12 +0100, Arnaud Ebalard wrote:
> Hi Eric,
>
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> After commit c9eeec26e32e ("tcp: TSQ can use a dynamic limit"), several
>> users reported throughput regressions, notably on mvneta and wifi
>> adapters.
>>
>> 802.11 AMPDU requires a fair amount of queueing to be effective.
>>
>> This patch partially reverts the change done in tcp_write_xmit() so that
>> the minimal amount is sysctl_tcp_limit_output_bytes.
>
> I just tested the fix on current linux tree with the same setup on which I
> observed the regression: it also fixes it on my side on my RN102. Thanks
> for the quick fix, Eric.
+1: fixes spastic NFS & Samba throughput since 3.12.0 with r8169 and e100e
for me as well. It's really not just broken/weird Wifi cards that are
affected by this.
Thanks!
-h
^ 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