* Re: [PATCH] net: Fix skb_under_panic oops in neigh_resolve_output
From: Eric Dumazet @ 2012-10-05 6:27 UTC (permalink / raw)
To: ramesh.nagappa
Cc: bnramesh, davem, edumazet, ebiederm, xemul, netdev, linux-kernel,
Ramesh Nagappa
In-Reply-To: <1349406325-3157-1-git-send-email-ramesh.nagappa@gmail.com>
On Thu, 2012-10-04 at 20:05 -0700, ramesh.nagappa@gmail.com wrote:
> From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
>
> The retry loop in neigh_resolve_output() and neigh_connected_output()
> call dev_hard_header() with out reseting the skb to network_header.
> This causes the retry to fail with skb_under_panic. The fix is to
> reset the network_header within the retry loop.
>
> Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
> Reviewed-by: Shawn Lu <shawn.lu@ericsson.com>
> Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>
> Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
> ---
> net/core/neighbour.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 117afaf..f50c542 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1313,6 +1313,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
>
> do {
> seq = read_seqbegin(&neigh->ha_lock);
> + __skb_pull(skb, skb_network_offset(skb));
This __skb_pull() could be done before the read_seqbegin() to keep the
protected section short.
> err = dev_hard_header(skb, dev, ntohs(skb->protocol),
> neigh->ha, NULL, skb->len);
> } while (read_seqretry(&neigh->ha_lock, seq));
> @@ -1342,10 +1343,9 @@ int neigh_connected_output(struct neighbour *neigh, struct sk_buff *skb)
> unsigned int seq;
> int err;
>
> - __skb_pull(skb, skb_network_offset(skb));
> -
> do {
> seq = read_seqbegin(&neigh->ha_lock);
> + __skb_pull(skb, skb_network_offset(skb));
This __skb_pull() could be done before the read_seqbegin() to keep the
protected section short.
> err = dev_hard_header(skb, dev, ntohs(skb->protocol),
> neigh->ha, NULL, skb->len);
> } while (read_seqretry(&neigh->ha_lock, seq));
Thanks
^ permalink raw reply
* [patch v4 1/2] netlink: add reference of module in netlink_dump_start
From: Gao feng @ 2012-10-05 6:15 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ,
pablo-Cap9r6Oaw4JrovVCs/uTlw, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng
I get a panic when I use ss -a and rmmod inet_diag at the
same time.
it's because netlink_dump use inet_diag_dump witch function
belongs to module inet_diag.
I search the codes and find many modules have the same problem.
We need add reference of the module which the cb->dump belongs
to.
Thanks for all help from Stephen,Jan,Eric,Steffen and Pablo.
Change From v3:
change netlink_dump_start to inline,suggestion from Pablo and
Eric.
Change From v2:
delete netlink_dump_done,and call module_put in netlink_dump
and netlink_sock_destruct.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
include/linux/netlink.h | 20 ++++++++++++++++----
net/netlink/af_netlink.c | 29 +++++++++++++++++++++--------
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index f80c56a..6d3af05 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -245,6 +245,8 @@ struct netlink_callback {
struct netlink_callback *cb);
int (*done)(struct netlink_callback *cb);
void *data;
+ /* the module that dump function belong to */
+ struct module *module;
u16 family;
u16 min_dump_alloc;
unsigned int prev_seq, seq;
@@ -262,14 +264,24 @@ __nlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, int type, int len, int fla
struct netlink_dump_control {
int (*dump)(struct sk_buff *skb, struct netlink_callback *);
- int (*done)(struct netlink_callback*);
+ int (*done)(struct netlink_callback *);
void *data;
+ struct module *module;
u16 min_dump_alloc;
};
-extern int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- struct netlink_dump_control *control);
+extern int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control);
+static inline int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control)
+{
+ if (!control->module)
+ control->module = THIS_MODULE;
+
+ return __netlink_dump_start(ssk, skb, nlh, control);
+}
#endif /* __KERNEL__ */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0f2e3ad..01e944a 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -169,6 +169,8 @@ static void netlink_sock_destruct(struct sock *sk)
if (nlk->cb) {
if (nlk->cb->done)
nlk->cb->done(nlk->cb);
+
+ module_put(nlk->cb->module);
netlink_destroy_callback(nlk->cb);
}
@@ -1758,6 +1760,7 @@ static int netlink_dump(struct sock *sk)
nlk->cb = NULL;
mutex_unlock(nlk->cb_mutex);
+ module_put(cb->module);
netlink_consume_callback(cb);
return 0;
@@ -1767,9 +1770,9 @@ errout_skb:
return err;
}
-int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- struct netlink_dump_control *control)
+int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control)
{
struct netlink_callback *cb;
struct sock *sk;
@@ -1784,6 +1787,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
cb->done = control->done;
cb->nlh = nlh;
cb->data = control->data;
+ cb->module = control->module;
cb->min_dump_alloc = control->min_dump_alloc;
atomic_inc(&skb->users);
cb->skb = skb;
@@ -1794,19 +1798,28 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
return -ECONNREFUSED;
}
nlk = nlk_sk(sk);
- /* A dump is in progress... */
+
mutex_lock(nlk->cb_mutex);
+ /* A dump is in progress... */
if (nlk->cb) {
mutex_unlock(nlk->cb_mutex);
netlink_destroy_callback(cb);
- sock_put(sk);
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
+ /* add reference of module which cb->dump belongs to */
+ if (!try_module_get(cb->module)) {
+ mutex_unlock(nlk->cb_mutex);
+ netlink_destroy_callback(cb);
+ ret = -EPROTONOSUPPORT;
+ goto out;
+ }
+
nlk->cb = cb;
mutex_unlock(nlk->cb_mutex);
ret = netlink_dump(sk);
-
+out:
sock_put(sk);
if (ret)
@@ -1817,7 +1830,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
*/
return -EINTR;
}
-EXPORT_SYMBOL(netlink_dump_start);
+EXPORT_SYMBOL(__netlink_dump_start);
void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
{
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 related
* Re: [PATCH 08/11] net: ll_temac: Do not use fixed mtu size
From: Michal Simek @ 2012-10-05 5:58 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
Rob Herring, David S. Miller
In-Reply-To: <1349378563.2817.27.camel@bwh-desktop.uk.solarflarecom.com>
On 10/04/2012 09:22 PM, Ben Hutchings wrote:
> On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
>> Use max mtu instead.
> [...]
>
> MTU does not include the Ethernet header so I have no idea how this is
> expected to work...
Right. This is wrong fix. It should be the same as is in axienet.
There is max_frm_size used which is mtu+header+tailer.
I will fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-05 5:43 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Michael S. Tsirkin, Thomas Lendacky, kvm, netdev, linux-kernel,
virtualization, avi, Sasha Levin
In-Reply-To: <506D8DD5.20904@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> Il 04/10/2012 14:51, Rusty Russell ha scritto:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> Il 04/10/2012 02:11, Rusty Russell ha scritto:
>>>>>> There's a reason I haven't done this. I really, really dislike "my
>>>>>> implemention isn't broken" feature bits. We could have an infinite
>>>>>> number of them, for each bug in each device.
>>>>>
>>>>> However, this bug affects (almost) all implementations and (almost) all
>>>>> devices. It even makes sense to reserve a transport feature bit for it
>>>>> instead of a device feature bit.
>>>>
>>>> Perhaps, but we have to fix the bugs first!
>>>
>>> Yes. :) Isn't that what mst's patch does?
>>>
>>>> As I said, my torture patch broke qemu immediately. Since noone has
>>>> leapt onto fixing that, I'll take a look now...
>>>
>>> I can look at virtio-scsi.
>>
>> Actually, you can't, see my reply to Anthony...
>>
>> Message-ID: <87lifm1y1n.fsf@rustcorp.com.au>
>
> struct virtio_scsi_req_cmd {
> // Read-only
> u8 lun[8];
> u64 id;
> u8 task_attr;
> u8 prio;
> u8 crn;
> char cdb[cdb_size];
> char dataout[];
> // Write-only part
> u32 sense_len;
> u32 residual;
> u16 status_qualifier;
> u8 status;
> u8 response;
> u8 sense[sense_size];
> char datain[];
> };
>
> where cdb_size and sense_size come from configuration space. The device
> right now expects everything before dataout/datain to be in a single
> descriptor, but that's in no way part of the spec. Am I missing
> something egregious?
Since you wrote it, I hope not :)
That's good. But virtio_blk's scsi command is insoluble AFAICT. As I
said to Anthony, the best rules are "always" and "never", so I'd really
rather not have to grandfather that in.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH 01/11] net: axienet: Remove NETIF_F_SG dropping for no checksum feature
From: Michal Simek @ 2012-10-05 5:22 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, anirudh, John.Linn, grant.likely,
rob.herring
In-Reply-To: <20121004.142612.1111166663063631532.davem@davemloft.net>
Hi David,
On 10/04/2012 08:26 PM, David Miller wrote:
>
> Sorry, no.
>
> I've announced on netdev very clearly that net-next submissions are not
> appropriate at this time and that only pure bug fixes should be submitted.
>
> Watch for the announcement on netdev of net-next openning up after the
> merge window closes, that's when you should resubmit this series.
Sorry I should label it as RFC.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Julia Lawall @ 2012-10-05 5:22 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, peter.senna, shemminger, mlindner, kernel-janitors,
netdev, linux-kernel
In-Reply-To: <1349395795.2008.26.camel@joe-AO722>
On Thu, 4 Oct 2012, Joe Perches wrote:
> On Thu, 2012-10-04 at 14:54 -0400, David Miller wrote:
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>>> On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
>>>> We want to know the implications of the bug being fixed.
>>>> Does it potentially cause an OOPS? Bad reference counting and thus
>>>> potential leaks or early frees?
>>>>
>>>> You have to analyze the implications and ramifications of the bug
>>>> being fixed. We need that information.
>
> You are asking for deeper level analysis that may not
> be reasonably possible from the robotic patch fixed
> by a robotic piece of a bit of code correction via a
> static analysis.
As Peter pointed out, it is not even a robotic fix, if robotic means
literally that it was done by a robot. A tool was used to find a
potential problem, and then Peter studied the code to see what fix was
appropriate.
In this case, finding out the impact, requires looking up the call chain
in all directions to find out what the callers do with the returned value.
And understanding the likely impact along the way. Often the call chain
involves function pointers. This is all possible to do. And perhaps it
would be even more helpful to the consumers of the patch than just having
the problem fixed. But the fact remains that at other places in the
function, someone thought it was useful to return an error code, so in the
place where the error code return is missing, it seems like a useful fault
to fix, whether it has an impact now or might have one later. The main
human analysis required to generate the patch is to be convinced that the
given return path really represents an error condition and that the
function normally returns the specified kind of value in that case. If
there is some way in which these points are unclear, then the commit
message should certainly explain the reasoning that was used.
julia
>
>>>> It's just "bad error code, this is the script that fixed it, kthx,
>>>> bye" which is pretty much useless for anaylsis.
>
> Which may be all but impossible but for the handful of
> folks that know all the gory/intimate details of the
> original bit of code.
>
>> What does it potentially cause the caller to do? Will it potentially
>> treat an error as a success and as a result register an object
>> illegally?
>>
>> Real analysis please. The text you provided above is basically still
>> robotic and could be used to describe any error code return fix.
>
> Right, it's useful to attempt but may be infeasible in
> practice.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" 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: Question: Proper place for port details?
From: Jeff Kirsher @ 2012-10-05 3:12 UTC (permalink / raw)
To: raspl; +Cc: netdev
In-Reply-To: <506D4896.8080900@linux.vnet.ibm.com>
On 10/04/2012 01:28 AM, Stefan Raspl wrote:
> Some network device drivers are capable of collecting further
> information on the link partner, e.g. reporting details on the port
> in the adjacent switch. Is there a good place to make these
> available outside of the kernel? ethtool gives a bit of information
> like the link partner's speed or duplex mode. But is adding further
> information about the link partner to ethtool desired, or is there a
> better place somewhere else?
>
> Ciao,
> Stefan
IMHO, reporting details on a port should be reported by Ethtool.
^ permalink raw reply
* [PATCH] net: Fix skb_under_panic oops in neigh_resolve_output
From: ramesh.nagappa @ 2012-10-05 3:05 UTC (permalink / raw)
To: bnramesh
Cc: davem, edumazet, ebiederm, xemul, netdev, linux-kernel,
Ramesh Nagappa
From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
The retry loop in neigh_resolve_output() and neigh_connected_output()
call dev_hard_header() with out reseting the skb to network_header.
This causes the retry to fail with skb_under_panic. The fix is to
reset the network_header within the retry loop.
Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
Reviewed-by: Shawn Lu <shawn.lu@ericsson.com>
Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>
Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
---
net/core/neighbour.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 117afaf..f50c542 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1313,6 +1313,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
do {
seq = read_seqbegin(&neigh->ha_lock);
+ __skb_pull(skb, skb_network_offset(skb));
err = dev_hard_header(skb, dev, ntohs(skb->protocol),
neigh->ha, NULL, skb->len);
} while (read_seqretry(&neigh->ha_lock, seq));
@@ -1342,10 +1343,9 @@ int neigh_connected_output(struct neighbour *neigh, struct sk_buff *skb)
unsigned int seq;
int err;
- __skb_pull(skb, skb_network_offset(skb));
-
do {
seq = read_seqbegin(&neigh->ha_lock);
+ __skb_pull(skb, skb_network_offset(skb));
err = dev_hard_header(skb, dev, ntohs(skb->protocol),
neigh->ha, NULL, skb->len);
} while (read_seqretry(&neigh->ha_lock, seq));
--
1.7.11.4
^ permalink raw reply related
* Re: [patch v3 01/11] netlink: add reference of module in netlink_dump_start
From: Gao feng @ 2012-10-05 0:50 UTC (permalink / raw)
To: Ben Hutchings
Cc: davem, eric.dumazet, steffen.klassert, netfilter-devel,
linux-rdma, netdev, linux-crypto, pablo, stephen.hemminger,
jengelh
In-Reply-To: <1349365302.2817.1.camel@bwh-desktop.uk.solarflarecom.com>
于 2012年10月04日 23:41, Ben Hutchings 写道:
> 'which' not 'witch' :-)
>
> Ben.
thanks Ben,will fix it :)
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Joe Perches @ 2012-10-05 0:09 UTC (permalink / raw)
To: David Miller
Cc: peter.senna, shemminger, mlindner, kernel-janitors, netdev,
linux-kernel
In-Reply-To: <20121004.145419.1859367129463136197.davem@davemloft.net>
On Thu, 2012-10-04 at 14:54 -0400, David Miller wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
> > On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
> >> We want to know the implications of the bug being fixed.
> >> Does it potentially cause an OOPS? Bad reference counting and thus
> >> potential leaks or early frees?
> >>
> >> You have to analyze the implications and ramifications of the bug
> >> being fixed. We need that information.
You are asking for deeper level analysis that may not
be reasonably possible from the robotic patch fixed
by a robotic piece of a bit of code correction via a
static analysis.
> >> It's just "bad error code, this is the script that fixed it, kthx,
> >> bye" which is pretty much useless for anaylsis.
Which may be all but impossible but for the handful of
folks that know all the gory/intimate details of the
original bit of code.
> What does it potentially cause the caller to do? Will it potentially
> treat an error as a success and as a result register an object
> illegally?
>
> Real analysis please. The text you provided above is basically still
> robotic and could be used to describe any error code return fix.
Right, it's useful to attempt but may be infeasible in
practice.
^ permalink raw reply
* [PATCH] net, TTY: initialize tty->driver_data before usage
From: Sasha Levin @ 2012-10-05 0:01 UTC (permalink / raw)
To: samuel, davem, gregkh, jslaby, alan
Cc: levinsasha928, davej, linux-kernel, netdev, Sasha Levin
Commit 9c650ffc ("TTY: ircomm_tty, add tty install") split _open() to
_install() and _open(). It also moved the initialization of driver_data
out of open(), but never added it to install() - causing a NULL ptr
deref whenever the driver was used.
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
net/irda/ircomm/ircomm_tty.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 95a3a7a..496ce2c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -421,6 +421,8 @@ static int ircomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
hashbin_insert(ircomm_tty, (irda_queue_t *) self, line, NULL);
}
+ tty->driver_data = self;
+
return tty_port_install(&self->port, driver, tty);
}
--
1.7.12
^ permalink raw reply related
* [PATCH] net, bluetooth: don't attempt to free a channel that wasn't created
From: Sasha Levin @ 2012-10-04 23:59 UTC (permalink / raw)
To: marcel, gustavo, johan.hedberg, davem
Cc: levinsasha928, davej, linux-kernel, linux-bluetooth, netdev,
Sasha Levin
We may currently attempt to free a channel which wasn't created due to
an error in the initialization path, this would cause a NULL ptr deref.
Introduced in commit 61d6ef3e ("Bluetooth: Make better use of l2cap_chan
reference counting").
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
net/bluetooth/l2cap_sock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 083f2bf..66c295a 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1083,7 +1083,8 @@ static void l2cap_sock_destruct(struct sock *sk)
{
BT_DBG("sk %p", sk);
- l2cap_chan_put(l2cap_pi(sk)->chan);
+ if (l2cap_pi(sk)->chan)
+ l2cap_chan_put(l2cap_pi(sk)->chan);
if (l2cap_pi(sk)->rx_busy_skb) {
kfree_skb(l2cap_pi(sk)->rx_busy_skb);
l2cap_pi(sk)->rx_busy_skb = NULL;
--
1.7.12
^ permalink raw reply related
* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 22:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <1349387874.21172.5.camel@edumazet-glaptop>
Nevermind, it depends on random being big enough. This shows that it
works with large enough random(). The hash I was getting from VXLAN
was too small...
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
int main(int ac, char **av) {
int low, high, remaining;
unsigned int rand;
unsigned short first;
low = atoi(av[1]);
high = atoi(av[2]);
remaining = (high - low) + 1;
srandom(getpid());
rand = (uint32_t) random();
first = (((uint64_t)rand * remaining) >> 32) + low;
printf("%d %d %u => %u\n", low, high, rand, first);
return 0;
}
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:59 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, dada1, netdev
In-Reply-To: <20121004.175009.1336502669507423764.davem@davemloft.net>
On Thu, 04 Oct 2012 17:50:09 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 04 Oct 2012 23:45:53 +0200
>
> > On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> >> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> >> David Miller <davem@davemloft.net> wrote:
> >>
> >> > From: Stephen Hemminger <shemminger@vyatta.com>
> >> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >> >
> >> > > While working on VXLAN, noticed a bug in UDP introduced by:
> >> > >
> >> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> >> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> >> > > Date: Wed Oct 8 11:44:17 2008 -0700
> >> > >
> >> > > udp: Improve port randomization
> >> > >
> >> > >
> >> > > The logic for choosing where to start for port randomization incorrectly
> >> > > calculates the starting port number. It is always ends up using
> >> > > the low end of the range independent of the value of random.
> >> > > This causes all UDP port searches to start at the same port.
> >> > >
> >> > > Doing the following fixes it but at the cost of doing a real divide.
> >> > >
> >> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >> > >
> >> > > ---
> >> > > Resend, previous send was not going to netdev.
> >> > >
> >> > > Not sure if worth fixing for stable, because only has performance impact
> >> > > and some application might be depending on current broken behaviour.
> >> > >
> >> > >
> >> > >
> >> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> >> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> >> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> >> > > remaining = (high - low) + 1;
> >> > >
> >> > > rand = net_random();
> >> > > - first = (((u64)rand * remaining) >> 32) + low;
> >> > > + first = rand % remaining + low;
> >> >
> >> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> >> > the expression instead.
> >>
> >> The standalone program gets same result.
> >
> > Hey, I hope you understand random32() does allocate a 32bit value, not a
> > 15bit one...
> >
> > If really we had such a bug, I am pretty sure we would have noticed.
>
> But the issue is is "remaining" that's of limited range, not rand.
> I didn't say to shift up 'rand', but rather 'remaining'.
I think the issue is the Gcc is deciding to truncate the math.
If it is done as separate steps, then the right answer happens:
Good:
t = ((uint64_t) rand * remaining) >> 32;
first = t + low;
Bad:
first = (((uint64_t)rand * remaining) >> (64-16)) + low;
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Eric Dumazet @ 2012-10-04 21:57 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, dada1, netdev
In-Reply-To: <20121004.175009.1336502669507423764.davem@davemloft.net>
On Thu, 2012-10-04 at 17:50 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 04 Oct 2012 23:45:53 +0200
>
> > On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> >> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> >> David Miller <davem@davemloft.net> wrote:
> >>
> >> > From: Stephen Hemminger <shemminger@vyatta.com>
> >> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >> >
> >> > > While working on VXLAN, noticed a bug in UDP introduced by:
> >> > >
> >> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> >> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> >> > > Date: Wed Oct 8 11:44:17 2008 -0700
> >> > >
> >> > > udp: Improve port randomization
> >> > >
> >> > >
> >> > > The logic for choosing where to start for port randomization incorrectly
> >> > > calculates the starting port number. It is always ends up using
> >> > > the low end of the range independent of the value of random.
> >> > > This causes all UDP port searches to start at the same port.
> >> > >
> >> > > Doing the following fixes it but at the cost of doing a real divide.
> >> > >
> >> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >> > >
> >> > > ---
> >> > > Resend, previous send was not going to netdev.
> >> > >
> >> > > Not sure if worth fixing for stable, because only has performance impact
> >> > > and some application might be depending on current broken behaviour.
> >> > >
> >> > >
> >> > >
> >> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> >> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> >> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> >> > > remaining = (high - low) + 1;
> >> > >
> >> > > rand = net_random();
> >> > > - first = (((u64)rand * remaining) >> 32) + low;
> >> > > + first = rand % remaining + low;
> >> >
> >> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> >> > the expression instead.
> >>
> >> The standalone program gets same result.
> >
> > Hey, I hope you understand random32() does allocate a 32bit value, not a
> > 15bit one...
> >
> > If really we had such a bug, I am pretty sure we would have noticed.
>
> But the issue is is "remaining" that's of limited range, not rand.
> I didn't say to shift up 'rand', but rather 'remaining'.
Sorry I see no issue.
remaining is the delta between high and low, and its right,
unless your compiler or cpu has a nice bug.
remaining = (high - low) + 1;
And I see no bug at all here.
The only assumption is that rand is a random number between 0 and
0xFFFFFFFF
$RANDOM is between 0 and 0x7FFF
Thats the difference
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:50 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, dada1, netdev
In-Reply-To: <1349387153.21172.1.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Oct 2012 23:45:53 +0200
> On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
>> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
>> David Miller <davem@davemloft.net> wrote:
>>
>> > From: Stephen Hemminger <shemminger@vyatta.com>
>> > Date: Thu, 4 Oct 2012 14:08:28 -0700
>> >
>> > > While working on VXLAN, noticed a bug in UDP introduced by:
>> > >
>> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
>> > > Author: Eric Dumazet <dada1@cosmosbay.com>
>> > > Date: Wed Oct 8 11:44:17 2008 -0700
>> > >
>> > > udp: Improve port randomization
>> > >
>> > >
>> > > The logic for choosing where to start for port randomization incorrectly
>> > > calculates the starting port number. It is always ends up using
>> > > the low end of the range independent of the value of random.
>> > > This causes all UDP port searches to start at the same port.
>> > >
>> > > Doing the following fixes it but at the cost of doing a real divide.
>> > >
>> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>> > >
>> > > ---
>> > > Resend, previous send was not going to netdev.
>> > >
>> > > Not sure if worth fixing for stable, because only has performance impact
>> > > and some application might be depending on current broken behaviour.
>> > >
>> > >
>> > >
>> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
>> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
>> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
>> > > remaining = (high - low) + 1;
>> > >
>> > > rand = net_random();
>> > > - first = (((u64)rand * remaining) >> 32) + low;
>> > > + first = rand % remaining + low;
>> >
>> > Try replacing "remaining" with "(remaining << (64 - 16))" in
>> > the expression instead.
>>
>> The standalone program gets same result.
>
> Hey, I hope you understand random32() does allocate a 32bit value, not a
> 15bit one...
>
> If really we had such a bug, I am pretty sure we would have noticed.
But the issue is is "remaining" that's of limited range, not rand.
I didn't say to shift up 'rand', but rather 'remaining'.
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Eric Dumazet @ 2012-10-04 21:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, dada1, netdev
In-Reply-To: <20121004142859.08ab0385@nehalam.linuxnetplumber.net>
On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >
> > > While working on VXLAN, noticed a bug in UDP introduced by:
> > >
> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> > > Date: Wed Oct 8 11:44:17 2008 -0700
> > >
> > > udp: Improve port randomization
> > >
> > >
> > > The logic for choosing where to start for port randomization incorrectly
> > > calculates the starting port number. It is always ends up using
> > > the low end of the range independent of the value of random.
> > > This causes all UDP port searches to start at the same port.
> > >
> > > Doing the following fixes it but at the cost of doing a real divide.
> > >
> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > >
> > > ---
> > > Resend, previous send was not going to netdev.
> > >
> > > Not sure if worth fixing for stable, because only has performance impact
> > > and some application might be depending on current broken behaviour.
> > >
> > >
> > >
> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> > > remaining = (high - low) + 1;
> > >
> > > rand = net_random();
> > > - first = (((u64)rand * remaining) >> 32) + low;
> > > + first = rand % remaining + low;
> >
> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> > the expression instead.
>
> The standalone program gets same result.
Hey, I hope you understand random32() does allocate a 32bit value, not a
15bit one...
If really we had such a bug, I am pretty sure we would have noticed.
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2012-10-04 21:42 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
The most important bit in here is the fix for input route caching
from Eric Dumazet, it's a shame we couldn't fully analyze this
in time for 3.6 as it's a 3.6 regression introduced by the routing
cache removal.
Anyways, will send quickly to -stable after you pull this in.
Other changes of note:
1) Fix lockdep splats in team and bonding, from Eric Dumazet.
2) IPV6 adds link local route even when there is no link local address,
from Nicolas Dichtel.
3) Fix ixgbe PTP implementation, from Jacob Keller.
4) Fix excessive stack usage in cxgb4 driver, from Vipul Pandya.
5) MAC length computed improperly in VLAN demux, from Antonio
Quartulli.
Please pull, thanks a lot!
The following changes since commit 7fe0b14b725d6d09a1d9e1409bd465cb88b587f9:
Merge tag 'spi-3.7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/misc (2012-10-02 17:26:42 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
for you to fetch changes up to 6825a26c2dc21eb4f8df9c06d3786ddec97cf53b:
ipv6: release reference of ip6_null_entry's dst entry in __ip6_del_rt (2012-10-04 16:00:07 -0400)
----------------------------------------------------------------
Antonio Quartulli (1):
8021q: fix mac_len recomputation in vlan_untag()
Dan Carpenter (1):
bnx2x: use strlcpy() to copy a string
Dave Jones (2):
silence some noisy printks in irda
Remove noisy printks from llcp_sock_connect
David S. Miller (4):
pch_gbe: Fix PTP dependencies.
cxgb4: Fix build error due to missing linux/vmalloc.h include.
Merge branch 'master' of git://git.kernel.org/.../jkirsher/net
Merge branch 'fixes-for-3.7' of git://gitorious.org/linux-can/linux-can
Emil Tantilov (1):
ixgbe: fix poll loop for FDIRCTRL.INIT_DONE bit
Eric Dumazet (3):
ipv4: add a fib_type to fib_info
bonding: set qdisc_tx_busylock to avoid LOCKDEP splat
team: set qdisc_tx_busylock to avoid LOCKDEP splat
Erik Hugne (1):
tipc: prevent dropped connections due to rcvbuf overflow
Gao feng (1):
ipv6: release reference of ip6_null_entry's dst entry in __ip6_del_rt
Jacob Keller (3):
ixgbe: Fix PTP X540 SDP alignment code for PPS signal
ixgbe: (PTP) Fix PPS interrupt code
ixgbe: fix PTP ethtool timestamping function
Marc Kleine-Budde (1):
can: mpc5xxx_can: fix section type conflict
Michael Chan (1):
tg3: Fix sparse warnings.
Nicolas Dichtel (3):
ipv6: don't add link local route when there is no link local address
sctp: fix a typo in prototype of __sctp_rcv_lookup()
sctp: check src addr when processing SACK to update transport state
Peter Senna Tschudin (2):
can: peak_pci: fix error return code
can: peak_pcmcia: fix error return code
Vipul Pandya (1):
cxgb4: Dynamically allocate memory in t4_memory_rw() and get_vpd_params()
Yuval Mintz (1):
bnx2x: fix ring size for 10G functions
htbegin (1):
net: ethernet: davinci_cpdma: decrease the desc count when cleaning up the remaining packets
joshua.a.hay@intel.com (1):
ixgbe: add support for X540-AT1
drivers/net/bonding/bond_main.c | 2 +
drivers/net/can/mscan/mpc5xxx_can.c | 8 +--
drivers/net/can/sja1000/peak_pci.c | 2 +
drivers/net/can/sja1000/peak_pcmcia.c | 4 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 17 ++++---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 5 +-
drivers/net/ethernet/broadcom/tg3.c | 11 +++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 +
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 43 +++++++++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 5 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 +++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 202 ++++++++++++++++++++++++++++++++++++----------------------------------------
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
drivers/net/ethernet/ti/davinci_cpdma.c | 1 +
drivers/net/team/team.c | 2 +
include/net/ip_fib.h | 1 +
include/net/sctp/structs.h | 2 +-
net/8021q/vlan_core.c | 3 +-
net/ipv4/fib_semantics.c | 2 +
net/ipv6/addrconf.c | 15 +-----
net/ipv6/route.c | 11 +++--
net/irda/af_irda.c | 2 +-
net/irda/irttp.c | 2 +-
net/nfc/llcp/sock.c | 8 +--
net/sctp/input.c | 2 +-
net/sctp/outqueue.c | 15 ++++--
net/sctp/sm_sideeffect.c | 4 +-
net/sctp/sm_statefuns.c | 2 +-
net/tipc/socket.c | 1 +
32 files changed, 209 insertions(+), 185 deletions(-)
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:28 UTC (permalink / raw)
To: David Miller; +Cc: dada1, netdev
In-Reply-To: <20121004.171246.1480276635491836767.davem@davemloft.net>
On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 4 Oct 2012 14:08:28 -0700
>
> > While working on VXLAN, noticed a bug in UDP introduced by:
> >
> > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> > Author: Eric Dumazet <dada1@cosmosbay.com>
> > Date: Wed Oct 8 11:44:17 2008 -0700
> >
> > udp: Improve port randomization
> >
> >
> > The logic for choosing where to start for port randomization incorrectly
> > calculates the starting port number. It is always ends up using
> > the low end of the range independent of the value of random.
> > This causes all UDP port searches to start at the same port.
> >
> > Doing the following fixes it but at the cost of doing a real divide.
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > ---
> > Resend, previous send was not going to netdev.
> >
> > Not sure if worth fixing for stable, because only has performance impact
> > and some application might be depending on current broken behaviour.
> >
> >
> >
> > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> > remaining = (high - low) + 1;
> >
> > rand = net_random();
> > - first = (((u64)rand * remaining) >> 32) + low;
> > + first = rand % remaining + low;
>
> Try replacing "remaining" with "(remaining << (64 - 16))" in
> the expression instead.
The standalone program gets same result.
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-04 21:27 UTC (permalink / raw)
To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349372079.16710.15.camel@sakura.staff.proxad.net>
On Thu, 2012-10-04 at 19:34 +0200, Maxime Bizon wrote:
> On Thu, 2012-10-04 at 19:17 +0200, Eric Dumazet wrote:
>
> > > yes, on ipv6 forward path the default NET_SKB_PAD is too small, so each
> > > packet forwarded has its headroom expanded, it is then recycled and gets
> > > its original default headroom back, then it gets forwarded,
> > > expanded, ...
> >
> > Hmm, this sounds bad (especially without recycle)
> >
> > Might I assume NET_SKB_PAD is 32 on this arch ?
>
> It is, I have a setup with 6to4 tunneling, so needed headroom on tx is
> quite big.
>
If we change NET_SKB_PAD minimum to be 64 (as it is on x86), it should
be enough for the added tunnel encapsulation or not ?
> I used to be careful about raising this value to avoid drivers using
> slab-4096 instead of slab-2048, but since our boards no longer have 16MB
> of RAM and with the recent changes in mainline it doesn't seem to be an
> issue anymore.
Yes, granted we can allocate order-3 pages for delivering skb->head
fragments, adding 32 bytes doesnt switch to slab-4096 since we dont use
it anymore.
>
> It's not a that big issue in the non recycle case, just lower
> performance if the tunable is not set correctly. Though it would be nice
> to have a stat/counter so you know when you hit this kind of slow path.
>
Yeah, we already mentioned this idea in the past. We are lazy now we
have good performance tools (perf)
> But on the recycle case, skb->head is reallocated to twice the size each
> time the packet is recycled and takes the same path again. This stresses
> the VM and you eventually get packet loss (and scary printk)
>
OK, so to fix this on stable trees, skb_recycle() should not recycle skb
if skb->head is too big.
By the way, another problem with skb_recycle() is that skb->truesize can
be wrong as well. (One skb might had one frag, with a truesize of
2048/4096 bytes, and this frag was pulled in skb->head, so skb->truesize
is slightly wrong.
So we also must check if skb->truesize is equal to
SKB_TRUESIZE(skb_end_offset(skb)), or reset it in skb_recycle(),
I have no strong opinion.
Something like this (untested) patch :
But I really think we should remove skb_recycle() when net-next is
opened again.
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b33a3a1..13ca215 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2659,7 +2659,10 @@ static inline bool skb_is_recycleable(const struct sk_buff *skb, int skb_size)
skb_size = SKB_DATA_ALIGN(skb_size + NET_SKB_PAD);
if (skb_end_offset(skb) < skb_size)
return false;
-
+ if (skb_end_offset(skb) > 2 * skb_size)
+ return false;
+ if (skb->truesize != SKB_TRUESIZE(skb_end_offset(skb)))
+ return false;
if (skb_shared(skb) || skb_cloned(skb))
return false;
^ permalink raw reply related
* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:12 UTC (permalink / raw)
To: shemminger; +Cc: dada1, netdev
In-Reply-To: <20121004140828.2d6f7bf9@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 4 Oct 2012 14:08:28 -0700
> While working on VXLAN, noticed a bug in UDP introduced by:
>
> commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> Author: Eric Dumazet <dada1@cosmosbay.com>
> Date: Wed Oct 8 11:44:17 2008 -0700
>
> udp: Improve port randomization
>
>
> The logic for choosing where to start for port randomization incorrectly
> calculates the starting port number. It is always ends up using
> the low end of the range independent of the value of random.
> This causes all UDP port searches to start at the same port.
>
> Doing the following fixes it but at the cost of doing a real divide.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> ---
> Resend, previous send was not going to netdev.
>
> Not sure if worth fixing for stable, because only has performance impact
> and some application might be depending on current broken behaviour.
>
>
>
> --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> remaining = (high - low) + 1;
>
> rand = net_random();
> - first = (((u64)rand * remaining) >> 32) + low;
> + first = rand % remaining + low;
Try replacing "remaining" with "(remaining << (64 - 16))" in
the expression instead.
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:12 UTC (permalink / raw)
To: stephen.hemminger; +Cc: dada1, netdev
In-Reply-To: <b400428e-ffe4-42ee-b656-99b0314d64f4@tahiti.vyatta.com>
From: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date: Thu, 04 Oct 2012 14:05:26 -0700 (PDT)
>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Thu, 4 Oct 2012 13:56:56 -0700
>>
>> > - first = (((u64)rand * remaining) >> 32) + low;
>> > + first = rand % remaining + low;
>>
>> I really don't get it, either a modulus via multiplcation
>> works or it doesn't. We have this construct all over the
>> place.
>>
>>
>
> Try out of kernel:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdint.h>
>
> int main(int ac, char **av) {
> int low, high, remaining;
> unsigned int rand;
> unsigned short first;
>
> low = atoi(av[1]);
> high = atoi(av[2]);
> remaining = (high - low) + 1;
> rand = atoi(av[3]);
>
> first = (((uint64_t)rand * remaining) >> 32) + low;
>
> printf("%d %d %u => %u\n", low, high, rand, first);
> return 0;
> }
>
> $ port-test 32768 61000 $RANDOM
> 32768 61000 7027 => 32768
It just shows that we need to shift "remaining" up into the top N
bits instead of the bottom N bits.
^ permalink raw reply
* [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:08 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev
While working on VXLAN, noticed a bug in UDP introduced by:
commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
Author: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed Oct 8 11:44:17 2008 -0700
udp: Improve port randomization
The logic for choosing where to start for port randomization incorrectly
calculates the starting port number. It is always ends up using
the low end of the range independent of the value of random.
This causes all UDP port searches to start at the same port.
Doing the following fixes it but at the cost of doing a real divide.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
Resend, previous send was not going to netdev.
Not sure if worth fixing for stable, because only has performance impact
and some application might be depending on current broken behaviour.
--- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
+++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
@@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
remaining = (high - low) + 1;
rand = net_random();
- first = (((u64)rand * remaining) >> 32) + low;
+ first = rand % remaining + low;
/*
* force rand to be an odd multiple of UDP_HTABLE_SIZE
*/
^ permalink raw reply
* RE: [PATCH 3/3] net/mlx4_en: Add HW timestamping (TS) support
From: Eugenia Emantayev @ 2012-10-04 20:58 UTC (permalink / raw)
To: Richard Cochran
Cc: Yevgeny Petrilin, davem@davemloft.net, netdev@vger.kernel.org,
Or Gerlitz
In-Reply-To: <20121004142734.GA2810@netboy.at.omicron.at>
> You could register the driver as PHC class device, described in
>
> Documentation/ptp/ptp.txt
>
> and then you device will work together along with a user space PTP stack, like the one I wrote at
>
> http://linuxptp.sourceforge.net
Richard,
Thanks for the explanation.
At this stage we prefer to submit the initial patch set (V1) and then, at the next phase,
extend it as you suggested above, after learning and better understanding the requirements
and effects.
Regards,
Eugenia
^ permalink raw reply
* Re: [PATCH] ipv6: release referenct of ip6_null_entry's dst entry in __ip6_del_rt
From: David Miller @ 2012-10-04 20:00 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <506CFA96.4050207@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 04 Oct 2012 10:55:18 +0800
> 于 2012年09月24日 12:32, Gao feng 写道:
>> 于 2012年09月22日 01:16, David Miller 写道:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>>
>>>> as we hold dst_entry before we call __ip6_del_rt,
>>>> so we should alse call dst_release not only return
>>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>>
>>>> and we already hold the dst entry, so I think it's
>>>> safe to call dst_release out of the write-read lock.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
...
> Can you apply this patch?
Applied and queued up for -stable, thanks.
^ 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