* Re: [iproute2 PATCH] iproute2: linklayer interface between kernel and tc/userspace
From: Stephen Hemminger @ 2013-09-03 15:33 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20130830120210.1510.80733.stgit@localhost>
On Fri, 30 Aug 2013 14:02:10 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> This iproute2 tc patch is connected to the kernel
> - commit 8a8e3d84b17 (net_sched: restore "linklayer atm" handling)
>
> The rate table calculated by tc, have gotten replaced in the kernel
> and is no-longer used for lookups.
>
> This happened in kernel release v3.8 caused by kernel
> - commit 56b765b79 ("htb: improved accuracy at high rates").
> This change unfortunately caused breakage of tc overhead and
> linklayer parameters.
>
> Kernel overhead handling got fixed in kernel v3.10 by
> - commit 01cb71d2d47 (net_sched: restore "overhead xxx" handling)
>
> Kernel linklayer handling got fixed in kernel v3.11 by
> - commit 8a8e3d84b17 (net_sched: restore "linklayer atm" handling)
>
> The linklayer fix introduced a struct change, that allow the linklayer
> attribute to be transferred between tc and kernel. This patch make use
> of this linklayer attribute.
>
> The linklayer setting is transfer to the kernel. And linklayer
> setting received from the kernel is printed with a prefixed
> "linklayer" when listing current configuration. The default
> TC_LINKLAYER_ETHERNET is only printed in detailed output mode.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Ok, Applied for 3.11
^ permalink raw reply
* Re: [PATCH v3] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)
From: Arvid Brodin @ 2013-09-03 15:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev, shemminger, joe, jboticario, balferreira
In-Reply-To: <20130830.155415.1756229438072680437.davem@davemloft.net>
On 2013-08-30 21:54, David Miller wrote:
> From: Arvid Brodin <arvid.brodin@xdin.com>
> Date: Wed, 21 Aug 2013 20:20:17 +0200
>
>> This is a patch against net-next (2013-08-21).
>
> First of all, your email client corrupted the patch, chopping up long lines
> and making whitespace changes to the content of the patch. Please fix this
> before resubmitting, and do so by first sending a test patch to yourself
> and making sure you can apply the patch you receive in that email. Do not
> use attachments to deal with this issue.
Sorry about that - I forgot to disable Format=Flowed... :(
>> + if (dev->operstate != transition) {
>> + write_lock_bh(&dev_base_lock);
>> + dev->operstate = transition;
>> + write_unlock_bh(&dev_base_lock);
>> + netdev_state_change(dev);
>> + }
>
> This is racy. And it appears that set_operstate() in net/core/rtnetlink.c
> has the same bug, I guess that's what you used as a guide.
>
> Any "test and set" sequence must be guarded completely by the lock, it
> doesn't make any sense to only guard the change of the value.
I never did understand the purpose of the locking done in set_operstate(). Will fix.
Also, Stephen Hemminger told me to use rtnl_mutex instead, but when I pointed
to net/core/rtnetlink.c and asked if he was sure, I did not get a reply. This
function - __hsr_set_operstate() - is only called from a notifier_call function,
so rtnl_mutex is already held here. Do I even need additional locking?
(According to ULNI* pp 171 Stephen is right and net/core/rtnetlink.c is wrong...)
* ULNI = Understanding Linux Network Internals
--
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.com
^ permalink raw reply
* Re: BQL support in gianfar causes network hiccup
From: Eric Dumazet @ 2013-09-03 15:42 UTC (permalink / raw)
To: Per Dalén; +Cc: Claudiu Manoil, netdev
In-Reply-To: <522595DD.1030600@appeartv.com>
On Tue, 2013-09-03 at 09:55 +0200, Per Dalén wrote:
> Yes, it's weird. The only reason I removed the BQL commit
> (d8a0f1b0af67679bba886784de10d8c21acc4e0e) was because the error Tino
> Keitel had was the similar as mine.
I suspect a genuine race in this driver. BQL only makes this race happen
more often.
gfar_poll_sq() has the following :
/* run Tx cleanup to completion */
if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx])
gfar_clean_tx_ring(tx_queue);
While gfar_poll() has a different method :
if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) {
gfar_clean_tx_ring(tx_queue);
has_tx_work = 1;
}
Note the has_tx_work use in gfar_poll() only.
Note that memory barriers seems to be missing.
1) In your cases, is it gfar_poll_sq() or gfar_poll() that is used ?
2) Is the bug happening if only one CPU is used ?
^ permalink raw reply
* [PATCH RFC] solos-pci: Fix BUG() with shared skb
From: David Woodhouse @ 2013-09-03 15:45 UTC (permalink / raw)
To: netdev; +Cc: Simon Arlott, linux-atm-general, nathan
[-- Attachment #1: Type: text/plain, Size: 2625 bytes --]
Simon reported this BUG():
kernel BUG at net/core/skbuff.c:1065!
Call Trace:
[<f9b7c12c>] ? pppoatm_send+0x3f/0x1a0 [pppoatm]
[<f8751797>] psend+0xa9/0x14a [solos_pci]
[<f9b7c248>] pppoatm_send+0x15b/0x1a0 [pppoatm]
[<f8a2f77d>] ppp_push+0x76/0x533 [ppp_generic]
(Rest of backtrace at http://s85.org/mn0aOxMN — the skb appears to be
IPv6, forwarded from another interface over PPPoATM.)
I wasn't expecting to see shared skbs in the ATM driver's ->send()
function. Is this the right fix?
I've included the whole function in the patch context... I appear to be
jumping through a lot of hand-coded hoops here, just to ensure that I
can safely prepend my own device's hardware header to the skb. This
makes me think that I'm doing something severely wrong. There ought to
be an easier way of doing this, surely? What am I missing?
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 32784d1..b15e475 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -1135,40 +1135,50 @@ static uint32_t fpga_tx(struct solos_card *card)
static int psend(struct atm_vcc *vcc, struct sk_buff *skb)
{
struct solos_card *card = vcc->dev->dev_data;
struct pkt_hdr *header;
int pktlen;
pktlen = skb->len;
if (pktlen > (BUF_SIZE - sizeof(*header))) {
dev_warn(&card->dev->dev, "Length of PDU is too large. Dropping PDU.\n");
solos_pop(vcc, skb);
return 0;
}
+ /* This is skb_share_check() except it uses solos_pop() instead of kfree_skb() */
+ if (skb_shared(skb)) {
+ struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!nskb)) {
+ solos_pop(vcc, skb);
+ return 0;
+ }
+ consume_skb(skb);
+ skb = nskb;
+ }
if (!skb_clone_writable(skb, sizeof(*header))) {
int expand_by = 0;
int ret;
if (skb_headroom(skb) < sizeof(*header))
expand_by = sizeof(*header) - skb_headroom(skb);
ret = pskb_expand_head(skb, expand_by, 0, GFP_ATOMIC);
if (ret) {
dev_warn(&card->dev->dev, "pskb_expand_head failed.\n");
solos_pop(vcc, skb);
return ret;
}
}
header = (void *)skb_push(skb, sizeof(*header));
/* This does _not_ include the size of the header */
header->size = cpu_to_le16(pktlen);
header->vpi = cpu_to_le16(vcc->vpi);
header->vci = cpu_to_le16(vcc->vci);
header->type = cpu_to_le16(PKT_DATA);
fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, vcc);
return 0;
}
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]
^ permalink raw reply related
* [PATCH net] bnx2x: bail out if unable to acquire stats_sema
From: Michal Schmidt @ 2013-09-03 15:45 UTC (permalink / raw)
To: davem; +Cc: netdev, Dmitry Kravkov, Ariel Elior, Eilon Greenstein
If we fail to acquire stats_sema in the specified time limit,
the chip is probably dead. It probably does not matter whether we
try to continue or not, but certainly we should not up() the semaphore
afterwards.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 24 +++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index 86436c7..d60bb8b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -538,16 +538,20 @@ static void __bnx2x_stats_start(struct bnx2x *bp)
static void bnx2x_stats_start(struct bnx2x *bp)
{
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
__bnx2x_stats_start(bp);
up(&bp->stats_sema);
}
static void bnx2x_stats_pmf_start(struct bnx2x *bp)
{
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
bnx2x_stats_comp(bp);
__bnx2x_stats_pmf_update(bp);
__bnx2x_stats_start(bp);
@@ -556,8 +560,10 @@ static void bnx2x_stats_pmf_start(struct bnx2x *bp)
static void bnx2x_stats_pmf_update(struct bnx2x *bp)
{
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
__bnx2x_stats_pmf_update(bp);
up(&bp->stats_sema);
}
@@ -569,8 +575,10 @@ static void bnx2x_stats_restart(struct bnx2x *bp)
*/
if (IS_VF(bp))
return;
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
bnx2x_stats_comp(bp);
__bnx2x_stats_start(bp);
up(&bp->stats_sema);
@@ -1361,8 +1369,10 @@ static void bnx2x_stats_stop(struct bnx2x *bp)
{
int update = 0;
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
bp->stats_started = false;
@@ -1997,8 +2007,10 @@ void bnx2x_afex_collect_stats(struct bnx2x *bp, void *void_afex_stats,
void bnx2x_stats_safe_exec(struct bnx2x *bp,
void (func_to_exec)(void *cookie),
void *cookie){
- if (down_timeout(&bp->stats_sema, HZ/10))
+ if (down_timeout(&bp->stats_sema, HZ/10)) {
BNX2X_ERR("Unable to acquire stats lock\n");
+ return;
+ }
bnx2x_stats_comp(bp);
func_to_exec(cookie);
__bnx2x_stats_start(bp);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH iproute2] pkt_sched: fq: Fair Queue packet scheduler
From: Stephen Hemminger @ 2013-09-03 15:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, ycheng, ncardwell
In-Reply-To: <1377829836.8277.59.camel@edumazet-glaptop>
On Thu, 29 Aug 2013 19:30:36 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Support for FQ packet scheduler
Applied to net-next-3.11 branch, with some minor cleanups.
^ permalink raw reply
* RE: [PATCH net] bnx2x: bail out if unable to acquire stats_sema
From: Dmitry Kravkov @ 2013-09-03 15:51 UTC (permalink / raw)
To: Michal Schmidt, davem@davemloft.net
Cc: netdev@vger.kernel.org, Ariel Elior, Eilon Greenstein
In-Reply-To: <1378223159-8710-1-git-send-email-mschmidt@redhat.com>
> -----Original Message-----
> From: Michal Schmidt [mailto:mschmidt@redhat.com]
> Sent: Tuesday, September 03, 2013 6:46 PM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Ariel Elior; Eilon Greenstein
> Subject: [PATCH net] bnx2x: bail out if unable to acquire stats_sema
>
> If we fail to acquire stats_sema in the specified time limit, the chip is
> probably dead. It probably does not matter whether we try to continue or
> not, but certainly we should not up() the semaphore afterwards.
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 24
> +++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
> index 86436c7..d60bb8b 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
> @@ -538,16 +538,20 @@ static void __bnx2x_stats_start(struct bnx2x *bp)
>
> static void bnx2x_stats_start(struct bnx2x *bp) {
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
> __bnx2x_stats_start(bp);
> up(&bp->stats_sema);
> }
>
> static void bnx2x_stats_pmf_start(struct bnx2x *bp) {
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
> bnx2x_stats_comp(bp);
> __bnx2x_stats_pmf_update(bp);
> __bnx2x_stats_start(bp);
> @@ -556,8 +560,10 @@ static void bnx2x_stats_pmf_start(struct bnx2x *bp)
>
> static void bnx2x_stats_pmf_update(struct bnx2x *bp) {
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
> __bnx2x_stats_pmf_update(bp);
> up(&bp->stats_sema);
> }
> @@ -569,8 +575,10 @@ static void bnx2x_stats_restart(struct bnx2x *bp)
> */
> if (IS_VF(bp))
> return;
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
> bnx2x_stats_comp(bp);
> __bnx2x_stats_start(bp);
> up(&bp->stats_sema);
> @@ -1361,8 +1369,10 @@ static void bnx2x_stats_stop(struct bnx2x *bp) {
> int update = 0;
>
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
>
> bp->stats_started = false;
>
> @@ -1997,8 +2007,10 @@ void bnx2x_afex_collect_stats(struct bnx2x *bp,
> void *void_afex_stats, void bnx2x_stats_safe_exec(struct bnx2x *bp,
> void (func_to_exec)(void *cookie),
> void *cookie){
> - if (down_timeout(&bp->stats_sema, HZ/10))
> + if (down_timeout(&bp->stats_sema, HZ/10)) {
> BNX2X_ERR("Unable to acquire stats lock\n");
> + return;
> + }
> bnx2x_stats_comp(bp);
> func_to_exec(cookie);
> __bnx2x_stats_start(bp);
> --
> 1.8.3.1
>
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
^ permalink raw reply
* Re: BQL support in gianfar causes network hiccup
From: Claudiu Manoil @ 2013-09-03 16:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Per Dalén, netdev
In-Reply-To: <1378222940.7360.34.camel@edumazet-glaptop>
On 9/3/2013 6:42 PM, Eric Dumazet wrote:
> On Tue, 2013-09-03 at 09:55 +0200, Per Dalén wrote:
>
>> Yes, it's weird. The only reason I removed the BQL commit
>> (d8a0f1b0af67679bba886784de10d8c21acc4e0e) was because the error Tino
>> Keitel had was the similar as mine.
>
>
> I suspect a genuine race in this driver. BQL only makes this race happen
> more often.
>
> gfar_poll_sq() has the following :
>
> /* run Tx cleanup to completion */
> if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx])
> gfar_clean_tx_ring(tx_queue);
>
> While gfar_poll() has a different method :
>
> if (tx_queue->tx_skbuff[tx_queue->skb_dirtytx]) {
> gfar_clean_tx_ring(tx_queue);
> has_tx_work = 1;
> }
>
> Note the has_tx_work use in gfar_poll() only.
>
> Note that memory barriers seems to be missing.
>
> 1) In your cases, is it gfar_poll_sq() or gfar_poll() that is used ?
>
It's gfar_poll_sq(). P2020 single Tx/Rx queues.
I'm also seeing carrier errors and packet collisions in this case
(100/Half link).
> 2) Is the bug happening if only one CPU is used ?
>
I didn't try this. Maybe Per did?
^ permalink raw reply
* [PATCH net-next] net: tcp_probe: adapt tbuf size for recent changes
From: Daniel Borkmann @ 2013-09-03 16:24 UTC (permalink / raw)
To: davem; +Cc: netdev
With recent changes in tcp_probe module (e.g. f925d0a62d ("net: tcp_probe:
add IPv6 support")) we also need to take into account that tbuf needs to
be updated as format string will be further expanded. tbuf sits on the stack
in tcpprobe_read() function that is invoked when user space reads procfs
file /proc/net/tcpprobe, hence not fast path as in jtcp_rcv_established().
Having a size similarly as in sctp_probe module of 256 bytes is fully
sufficient for that, we need theoretical maximum of 252 bytes otherwise we
could get truncated.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/ipv4/tcp_probe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_probe.c b/net/ipv4/tcp_probe.c
index 622a437..1f6aa54 100644
--- a/net/ipv4/tcp_probe.c
+++ b/net/ipv4/tcp_probe.c
@@ -218,7 +218,7 @@ static ssize_t tcpprobe_read(struct file *file, char __user *buf,
return -EINVAL;
while (cnt < len) {
- char tbuf[164];
+ char tbuf[256];
int width;
/* Wait for data in buffer */
--
1.7.11.7
^ permalink raw reply related
* RE: [patch] qlcnic: remove a stray semicolon
From: Himanshu Madhani @ 2013-09-03 16:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: Rajesh Borundia, Shahed Shaikh, Jitendra Kalsaria, Sony Chacko,
Sucheta Chakraborty, Dept-Eng Linux Driver, netdev,
kernel-janitors@vger.kernel.org
In-Reply-To: <20130903091347.GA4630@elgon.mountain>
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Tuesday, September 03, 2013 2:14 AM
> To: Himanshu Madhani
> Cc: Rajesh Borundia; Shahed Shaikh; Jitendra Kalsaria; Sony Chacko; Sucheta
> Chakraborty; Dept-Eng Linux Driver; netdev; kernel-janitors@vger.kernel.org
> Subject: [patch] qlcnic: remove a stray semicolon
>
> Just remove a small semicolon.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> index 332aa71..4d7ad00 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> @@ -1709,7 +1709,7 @@ qlcnic_set_dump(struct net_device *netdev, struct
> ethtool_dump *val)
> qlcnic_dev_request_reset(adapter, val->flag);
> adapter->flags &= ~QLCNIC_FW_RESET_OWNER;
> break;
> -;
> +
> case QLCNIC_SET_QUIESCENT:
> case QLCNIC_RESET_QUIESCENT:
> state = QLC_SHARED_REG_RD32(adapter,
> QLCNIC_CRB_DEV_STATE);
Thanks
Acked-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
^ permalink raw reply
* [PATCH net-next] vxlan: Optimize vxlan rcv
From: Pravin B Shelar @ 2013-09-03 16:44 UTC (permalink / raw)
To: netdev; +Cc: stephen, Pravin B Shelar
vxlan-udp-recv function lookup vxlan_sock struct on every packet
recv by using udp-port number. we can use sk->sk_user_data to
store vxlan_sock and avoid lookup.
I have open coded rcu-api to store and read vxlan_sock from
sk_user_data to avoid sparse warning as sk_user_data is not
__rcu pointer.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
drivers/net/vxlan.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 6b560f3..7de131f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -916,6 +916,8 @@ void vxlan_sock_release(struct vxlan_sock *vs)
spin_lock(&vn->sock_lock);
hlist_del_rcu(&vs->hlist);
+ smp_wmb();
+ vs->sock->sk->sk_user_data = NULL;
spin_unlock(&vn->sock_lock);
queue_work(vxlan_wq, &vs->del_work);
@@ -1009,7 +1011,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
port = inet_sk(sk)->inet_sport;
- vs = vxlan_find_sock(sock_net(sk), port);
+ smp_read_barrier_depends();
+ vs = (struct vxlan_sock *)sk->sk_user_data;
if (!vs)
goto drop;
@@ -2236,6 +2239,8 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
atomic_set(&vs->refcnt, 1);
vs->rcv = rcv;
vs->data = data;
+ smp_wmb();
+ vs->sock->sk->sk_user_data = vs;
spin_lock(&vn->sock_lock);
hlist_add_head_rcu(&vs->hlist, vs_head(net, port));
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
From: Neil Horman @ 2013-09-03 16:46 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: linux-sctp, davem, vyasevich,
Glavinic-Pecotic, Matija (EXT-Other - DE/Ulm), netdev
In-Reply-To: <52249981.8040006@nsn.com>
On Mon, Sep 02, 2013 at 03:58:25PM +0200, Alexander Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@nsn.com>
>
> net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
>
> Initially the problem was observed with ipsec, but later it became clear that
> SCTP data chunk fragmentation algorithm has problems with MTU values which are
> not multiple of 4. Test program was used which just transmits 2000 bytes long
> packets to other host. tcpdump was used to observe re-fragmentation in IP layer
> after SCTP already fragmented data chunks.
>
> With MTU 1500:
> 12:54:34.082904 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 1500)
> 10.151.38.153.39303 > 10.151.24.91.54321: sctp (1) [DATA] (B) [TSN: 2366088589] [SID: 0] [SSEQ 1] [PPID 0x0]
> 12:54:34.082933 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 596)
> 10.151.38.153.39303 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 2366088590] [SID: 0] [SSEQ 1] [PPID 0x0]
> 12:54:34.090576 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.39303: sctp (1) [SACK] [cum ack 2366088590] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> With MTU 1499:
> 13:02:49.955220 IP (tos 0x2,ECT(0), ttl 64, id 48215, offset 0, flags [+], proto SCTP (132), length 1492)
> 10.151.38.153.39084 > 10.151.24.91.54321: sctp[|sctp]
> 13:02:49.955249 IP (tos 0x2,ECT(0), ttl 64, id 48215, offset 1472, flags [none], proto SCTP (132), length 28)
> 10.151.38.153 > 10.151.24.91: ip-proto-132
> 13:02:49.955262 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 600)
> 10.151.38.153.39084 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 404355346] [SID: 0] [SSEQ 1] [PPID 0x0]
> 13:02:49.956770 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.39084: sctp (1) [SACK] [cum ack 404355346] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> Here problem in data portion limit calculation leads to re-fragmentation in IP,
> which is sub-optimal. The problem is max_data initial value, which doesn't take
> into account the fact, that data chunk must be padded to 4-bytes boundary.
> It's enough to correct max_data, because all later adjustments are correctly
> aligned to 4-bytes boundary.
>
> After the fix is applied, everything is fragmented correctly for uneven MTUs:
> 15:16:27.083881 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 1496)
> 10.151.38.153.53417 > 10.151.24.91.54321: sctp (1) [DATA] (B) [TSN: 3077098183] [SID: 0] [SSEQ 1] [PPID 0x0]
> 15:16:27.083907 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 600)
> 10.151.38.153.53417 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 3077098184] [SID: 0] [SSEQ 1] [PPID 0x0]
> 15:16:27.085640 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.53417: sctp (1) [SACK] [cum ack 3077098184] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> The bug was there for years already, but
> - is a performance issue, the packets are still transmitted
> - doesn't show up with default MTU 1500, but possibly with ipsec (MTU 1438)
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
> ---
>
> --- linux-3.10.orig/net/sctp/chunk.c
> +++ linux-3.10/net/sctp/chunk.c
> @@ -200,9 +200,9 @@ struct sctp_datamsg *sctp_datamsg_from_u
> /* This is the biggest possible DATA chunk that can fit into
> * the packet
> */
> - max_data = asoc->pathmtu -
> + max_data = (asoc->pathmtu -
> sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> - sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
> + sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk)) & ~3;
>
> max = asoc->frag_point;
> /* If the the peer requested that we authenticate DATA chunks
> --
> 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
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 17:19 UTC (permalink / raw)
To: Thomas Graf
Cc: davem, netdev, Eric Dumazet, Hannes Frederic Sowa, Fabio Estevam
In-Reply-To: <e4600dac657e1160da7a5e7758dcb973b616a10e.1378207925.git.tgraf@suug.ch>
On 09/03/2013 05:37 AM, Thomas Graf wrote:
> Allocating skbs when sending out neighbour discovery messages
> currently uses sock_alloc_send_skb() based on a per net namespace
> socket and thus share a socket wmem buffer space.
>
> If a netdevice is temporarily unable to transmit due to carrier
> loss or for other reasons, the queued up ndisc messages will cosnume
> all of the wmem space and will thus prevent from any more skbs to
> be allocated even for netdevices that are able to transmit packets.
>
> The number of neighbour discovery messages sent is very limited,
> use of alloc_skb() bypasses the socket wmem buffer size enforcement
> while the manual call to skb_set_owner_w() maintains the socket
> reference needed for the IPv6 output path.
>
> This patch has orginally been posted by Eric Dumazet in a modified
> form.
Tested-by: Stephen Warren <swarren@nvidia.com>
Although I do note something slightly odd:
next-20130830 had an issue, and reverting V1 of this patch solved it.
However, in next-20130903, if I revert the revert of V1 of this patch, I
don't see any issue; it appears that the problem was some interaction
between V1 of this patch and something else in next-20130830.
Either way, this patch doesn't seem to introduce any issue when applied
on top of either next-20130830 with V1 reverted, or on top of
next-20130903, so it's fine.
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Hannes Frederic Sowa @ 2013-09-03 17:27 UTC (permalink / raw)
To: Stephen Warren; +Cc: Thomas Graf, davem, netdev, Eric Dumazet, Fabio Estevam
In-Reply-To: <52261A12.3060203@wwwdotorg.org>
On Tue, Sep 03, 2013 at 11:19:14AM -0600, Stephen Warren wrote:
> On 09/03/2013 05:37 AM, Thomas Graf wrote:
> > Allocating skbs when sending out neighbour discovery messages
> > currently uses sock_alloc_send_skb() based on a per net namespace
> > socket and thus share a socket wmem buffer space.
> >
> > If a netdevice is temporarily unable to transmit due to carrier
> > loss or for other reasons, the queued up ndisc messages will cosnume
> > all of the wmem space and will thus prevent from any more skbs to
> > be allocated even for netdevices that are able to transmit packets.
> >
> > The number of neighbour discovery messages sent is very limited,
> > use of alloc_skb() bypasses the socket wmem buffer size enforcement
> > while the manual call to skb_set_owner_w() maintains the socket
> > reference needed for the IPv6 output path.
> >
> > This patch has orginally been posted by Eric Dumazet in a modified
> > form.
>
> Tested-by: Stephen Warren <swarren@nvidia.com>
>
> Although I do note something slightly odd:
>
> next-20130830 had an issue, and reverting V1 of this patch solved it.
>
> However, in next-20130903, if I revert the revert of V1 of this patch, I
> don't see any issue; it appears that the problem was some interaction
> between V1 of this patch and something else in next-20130830.
>
> Either way, this patch doesn't seem to introduce any issue when applied
> on top of either next-20130830 with V1 reverted, or on top of
> next-20130903, so it's fine.
Could either of you run the v1 version of the patch with CONFIG_PROVE_LOCKING
enabled? I also do think there is some side-effect we don't understand yet.
Thanks,
Hannes
^ permalink raw reply
* [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Daniel Borkmann @ 2013-09-03 17:29 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Dumazet
In tcp_v6_do_rcv() code, when processing pkt options, we soley work
on our skb clone opt_skb that we've created earlier before entering
tcp_rcv_established() on our way. However, only in condition ...
if (np->rxopt.bits.rxtclass)
np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
... we work on skb itself. As we extract every other information out
of opt_skb in ipv6_pktoptions path, this seems wrong, since skb can
already be released by tcp_rcv_established() earlier on. When we try
to access it in ipv6_hdr(), we will dereference freed skb.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/tcp_ipv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6e1649d..eeb4cb0 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1427,7 +1427,7 @@ ipv6_pktoptions:
if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
if (np->rxopt.bits.rxtclass)
- np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
+ np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(opt_skb));
if (ipv6_opt_accepted(sk, opt_skb)) {
skb_set_owner_r(opt_skb, sk);
opt_skb = xchg(&np->pktoptions, opt_skb);
--
1.7.11.7
^ permalink raw reply related
* [PATCH] xen-netback: count number required slots for an skb more carefully
From: David Vrabel @ 2013-09-03 17:29 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, Boris Ostrovsky,
Ian Campbell, netdev
From: David Vrabel <david.vrabel@citrix.com>
When a VM is providing an iSCSI target and the LUN is used by the
backend domain, the generated skbs for direct I/O writes to the disk
have large, multi-page skb->data but no frags.
With some lengths and starting offsets, xen_netbk_count_skb_slots()
would be one short because the simple calculation of
DIV_ROUND_UP(skb_headlen(), PAGE_SIZE) was not accounting for the
decisions made by start_new_rx_buffer() which does not guarantee
responses are fully packed.
For example, a skb with length < 2 pages but which spans 3 pages would
be counted as requiring 2 slots but would actually use 3 slots.
skb->data:
| 1111|222222222222|3333 |
Fully packed, this would need 2 slots:
|111122222222|22223333 |
But because the 2nd page wholy fits into a slot it is not split across
slots and goes into a slot of its own:
|1111 |222222222222|3333 |
Miscounting the number of slots means netback may push more responses
than the number of available requests. This will cause the frontend
to get very confused and report "Too many frags/slots". The frontend
never recovers and will eventually BUG.
Fix this by counting the number of required slots more carefully. In
xen_netbk_count_skb_slots(), more closely follow the algorithm used by
xen_netbk_gop_skb() by introducing xen_netbk_count_frag_slots() which
is the dry-run equivalent of netbk_gop_frag_copy().
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
[Resend to actually Cc netdev, sorry.]
---
drivers/net/xen-netback/netback.c | 94 +++++++++++++++++++++++++------------
1 files changed, 64 insertions(+), 30 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 64828de..f149ce5 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -361,6 +361,49 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
return false;
}
+struct xen_netbk_count_slot_state {
+ unsigned long copy_off;
+ bool head;
+};
+
+unsigned int xen_netbk_count_frag_slots(struct xenvif *vif,
+ unsigned long offset, unsigned long size,
+ struct xen_netbk_count_slot_state *state)
+{
+ unsigned count = 0;
+
+ offset &= ~PAGE_MASK;
+
+ while (size > 0) {
+ unsigned long bytes;
+
+ bytes = PAGE_SIZE - offset;
+
+ if (bytes > size)
+ bytes = size;
+
+ if (start_new_rx_buffer(state->copy_off, bytes, state->head)) {
+ count++;
+ state->copy_off = 0;
+ }
+
+ if (state->copy_off + bytes > MAX_BUFFER_OFFSET)
+ bytes = MAX_BUFFER_OFFSET - state->copy_off;
+
+ state->copy_off += bytes;
+
+ offset += bytes;
+ size -= bytes;
+
+ if (offset == PAGE_SIZE)
+ offset = 0;
+
+ state->head = false;
+ }
+
+ return count;
+}
+
/*
* Figure out how many ring slots we're going to need to send @skb to
* the guest. This function is essentially a dry run of
@@ -368,48 +411,39 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
*/
unsigned int xen_netbk_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
{
+ struct xen_netbk_count_slot_state state;
unsigned int count;
- int i, copy_off;
+ unsigned char *data;
+ unsigned i;
- count = DIV_ROUND_UP(skb_headlen(skb), PAGE_SIZE);
+ state.head = true;
+ state.copy_off = 0;
- copy_off = skb_headlen(skb) % PAGE_SIZE;
+ /* Slot for the first (partial) page of data. */
+ count = 1;
+ /* Need a slot for the GSO prefix for GSO extra data? */
if (skb_shinfo(skb)->gso_size)
count++;
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
- unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
- unsigned long bytes;
-
- offset &= ~PAGE_MASK;
-
- while (size > 0) {
- BUG_ON(offset >= PAGE_SIZE);
- BUG_ON(copy_off > MAX_BUFFER_OFFSET);
-
- bytes = PAGE_SIZE - offset;
-
- if (bytes > size)
- bytes = size;
+ data = skb->data;
+ while (data < skb_tail_pointer(skb)) {
+ unsigned long offset = offset_in_page(data);
+ unsigned long size = PAGE_SIZE - offset;
- if (start_new_rx_buffer(copy_off, bytes, 0)) {
- count++;
- copy_off = 0;
- }
+ if (data + size > skb_tail_pointer(skb))
+ size = skb_tail_pointer(skb) - data;
- if (copy_off + bytes > MAX_BUFFER_OFFSET)
- bytes = MAX_BUFFER_OFFSET - copy_off;
+ count += xen_netbk_count_frag_slots(vif, offset, size, &state);
- copy_off += bytes;
+ data += size;
+ }
- offset += bytes;
- size -= bytes;
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
- if (offset == PAGE_SIZE)
- offset = 0;
- }
+ count += xen_netbk_count_frag_slots(vif, offset, size, &state);
}
return count;
}
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
From: David Daney @ 2013-09-03 17:32 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: David Daney, netdev
In-Reply-To: <1378071184.25743.52.camel@deadeye.wl.decadent.org.uk>
On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> bcm87xx currently isn't auto-loaded if built as a module.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Compile-tested only.
Then how do you know that it does anything sensible?
Other than that, it seems plausible.
David Daney
>
> Ben.
>
> drivers/net/phy/bcm87xx.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> index 7997895..f5b44ff 100644
> --- a/drivers/net/phy/bcm87xx.c
> +++ b/drivers/net/phy/bcm87xx.c
> @@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
> }
> module_exit(bcm87xx_exit);
>
> +static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
> + { PHY_ID_BCM8706, 0xffffffff },
> + { PHY_ID_BCM8727, 0xffffffff },
> + { }
> +};
> +
> MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 17:42 UTC (permalink / raw)
To: Thomas Graf, davem, netdev, Eric Dumazet, Fabio Estevam
In-Reply-To: <20130903172736.GB21729@order.stressinduktion.org>
On 09/03/2013 11:27 AM, Hannes Frederic Sowa wrote:
> On Tue, Sep 03, 2013 at 11:19:14AM -0600, Stephen Warren wrote:
>> On 09/03/2013 05:37 AM, Thomas Graf wrote:
>>> Allocating skbs when sending out neighbour discovery messages
>>> currently uses sock_alloc_send_skb() based on a per net namespace
>>> socket and thus share a socket wmem buffer space.
>>>
>>> If a netdevice is temporarily unable to transmit due to carrier
>>> loss or for other reasons, the queued up ndisc messages will cosnume
>>> all of the wmem space and will thus prevent from any more skbs to
>>> be allocated even for netdevices that are able to transmit packets.
>>>
>>> The number of neighbour discovery messages sent is very limited,
>>> use of alloc_skb() bypasses the socket wmem buffer size enforcement
>>> while the manual call to skb_set_owner_w() maintains the socket
>>> reference needed for the IPv6 output path.
>>>
>>> This patch has orginally been posted by Eric Dumazet in a modified
>>> form.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> Although I do note something slightly odd:
>>
>> next-20130830 had an issue, and reverting V1 of this patch solved it.
>>
>> However, in next-20130903, if I revert the revert of V1 of this patch, I
>> don't see any issue; it appears that the problem was some interaction
>> between V1 of this patch and something else in next-20130830.
>>
>> Either way, this patch doesn't seem to introduce any issue when applied
>> on top of either next-20130830 with V1 reverted, or on top of
>> next-20130903, so it's fine.
>
> Could either of you run the v1 version of the patch with CONFIG_PROVE_LOCKING
> enabled? I also do think there is some side-effect we don't understand yet.
I don't see any extra messages from PROVE_LOCKING related to networking.
There is a single extra message from inside the audio driver, but that's
not networking-related at all.
^ permalink raw reply
* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Eric Dumazet @ 2013-09-03 17:48 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, Jiri Benc
In-Reply-To: <1378229352-9779-1-git-send-email-dborkman@redhat.com>
On Tue, 2013-09-03 at 19:29 +0200, Daniel Borkmann wrote:
> In tcp_v6_do_rcv() code, when processing pkt options, we soley work
> on our skb clone opt_skb that we've created earlier before entering
> tcp_rcv_established() on our way. However, only in condition ...
>
> if (np->rxopt.bits.rxtclass)
> np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
>
> ... we work on skb itself. As we extract every other information out
> of opt_skb in ipv6_pktoptions path, this seems wrong, since skb can
> already be released by tcp_rcv_established() earlier on. When we try
> to access it in ipv6_hdr(), we will dereference freed skb.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/ipv6/tcp_ipv6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6e1649d..eeb4cb0 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1427,7 +1427,7 @@ ipv6_pktoptions:
> if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
> np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
> if (np->rxopt.bits.rxtclass)
> - np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
> + np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(opt_skb));
> if (ipv6_opt_accepted(sk, opt_skb)) {
> skb_set_owner_r(opt_skb, sk);
> opt_skb = xchg(&np->pktoptions, opt_skb);
Bug added in commit 4c507d2897bd9b
("net: implement IP_RECVTOS for IP_PKTOPTIONS")
CC Jiri
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Eric Dumazet @ 2013-09-03 17:51 UTC (permalink / raw)
To: Stephen Warren; +Cc: Thomas Graf, davem, netdev, Fabio Estevam
In-Reply-To: <52261F68.70907@wwwdotorg.org>
On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
> I don't see any extra messages from PROVE_LOCKING related to networking.
> There is a single extra message from inside the audio driver, but that's
> not networking-related at all.
LOCKDEP is automatically disabled at first splat.
Please try a kernel without audio driver ;)
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
From: Hannes Frederic Sowa @ 2013-09-03 17:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, David Stevens
In-Reply-To: <1378195178-21002-2-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:31AM +0200, Daniel Borkmann wrote:
> i) RFC3810, 9.2. Query Interval [QI] says:
>
> The Query Interval variable denotes the interval between General
> Queries sent by the Querier. Default value: 125 seconds. [...]
>
> ii) RFC3810, 9.3. Query Response Interval [QRI] says:
>
> The Maximum Response Delay used to calculate the Maximum Response
> Code inserted into the periodic General Queries. Default value:
> 10000 (10 seconds) [...] The number of seconds represented by the
> [Query Response Interval] must be less than the [Query Interval].
>
> iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:
>
> The Older Version Querier Present Timeout is the time-out for
> transitioning a host back to MLDv2 Host Compatibility Mode. When an
> MLDv1 query is received, MLDv2 hosts set their Older Version Querier
> Present Timer to [Older Version Querier Present Timeout].
>
> This value MUST be ([Robustness Variable] times (the [Query Interval]
> in the last Query received)) plus ([Query Response Interval]).
>
> Hence, on *default* the timeout results in:
>
> [RV] = 2, [QI] = 125sec, [QRI] = 10sec
> [OVQPT] = [RV] * [QI] + [QRI] = 260sec
>
> Having that said, we currently calculate [OVQPT] (here given as 'switchback'
> variable) as ...
>
> switchback = (idev->mc_qrv + 1) * max_delay
>
> RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
> section "9.14. Configuring timers", it is said:
>
> This section is meant to provide advice to network administrators on
> how to tune these settings to their network. Ambitious router
> implementations might tune these settings dynamically based upon
> changing characteristics of the network. [...]
>
> iv) RFC38010, 9.14.2. Query Interval:
>
> The overall level of periodic MLD traffic is inversely proportional
> to the Query Interval. A longer Query Interval results in a lower
> overall level of MLD traffic. The value of the Query Interval MUST
> be equal to or greater than the Maximum Response Delay used to
> calculate the Maximum Response Code inserted in General Query
> messages.
>
> I assume that was why switchback is calculated as is (3 * max_delay), although
> this setting seems to be meant for routers only to configure their [QI]
> interval for non-default intervals. So usage here like this is clearly wrong.
>
> Concluding, the current behaviour in IPv6's multicast code is not conform
> to the RFC as switch back is calculated wrongly. That is, it has a too small
> value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
> instead of ~260secs on default.
>
> Hence, introduce necessary helper functions and fix this up properly as it
> should be.
>
> Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
> Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
> who did initial testing.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: David Stevens <dlstevens@us.ibm.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
From: Hannes Frederic Sowa @ 2013-09-03 17:55 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-3-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:32AM +0200, Daniel Borkmann wrote:
> Replace the macro with a function to make it more readable. GCC will
> eventually decide whether to inline this or not (also, that's not
> fast-path anyway).
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: VLAN HW accel, performance advantage?
From: Joakim Tjernlund @ 2013-09-03 18:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, netdev
In-Reply-To: <1378222150.7360.28.camel@edumazet-glaptop>
Eric Dumazet <eric.dumazet@gmail.com> wrote on 2013/09/03 17:29:10:
>
> On Tue, 2013-09-03 at 15:48 +0100, Ben Hutchings wrote:
>
> > GRO should be beneficial for bridges if you have bursty TCP streams.
It
> > is especially beneficial if you also have TSO on the TX side.
> >
> > Really you're going to have to try it and run benchmarks with your own
> > hardware.
>
> Note that ucc_geth does not have GRO support (it calls
> netif_receive_skb())
>
> It doesnt have RX checksuming either, so having to validate
> tcp checksums (before aggregating them in GRO stack) would hurt if
> packets only have to be forwarded.
Yes, the controller only have IP header checksum. Seems there is little to
gain with HW VLAN support for this controller, maybe when I got some spare
time
then. :)
Thanks guys
Jocke
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 18:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Thomas Graf, davem, netdev, Fabio Estevam
In-Reply-To: <1378230697.7360.38.camel@edumazet-glaptop>
On 09/03/2013 11:51 AM, Eric Dumazet wrote:
> On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
>
>> I don't see any extra messages from PROVE_LOCKING related to networking.
>> There is a single extra message from inside the audio driver, but that's
>> not networking-related at all.
>
> LOCKDEP is automatically disabled at first splat.
>
> Please try a kernel without audio driver ;)
Ah, OK. Now I do see something from ipv6:
> [ 25.327622]
> [ 25.329142] =============================================
> [ 25.334533] [ INFO: possible recursive locking detected ]
> [ 25.339927] 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17 Not tainted
> [ 25.346705] ---------------------------------------------
> [ 25.352095] login/704 is trying to acquire lock:
> [ 25.356705] (&ndev->lock){++--..}, at: [<c049be24>] ipv6_chk_mcast_addr+0x5c/0x200
> [ 25.364405]
> [ 25.364405] but task is already holding lock:
> [ 25.370230] (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
> [ 25.377741]
> [ 25.377741] other info that might help us debug this:
> [ 25.384261] Possible unsafe locking scenario:
> [ 25.384261]
> [ 25.390174] CPU0
> [ 25.392614] ----
> [ 25.395053] lock(&ndev->lock);
> [ 25.398286] lock(&ndev->lock);
> [ 25.401518]
> [ 25.401518] *** DEADLOCK ***
> [ 25.401518]
> [ 25.407433] May be due to missing lock nesting notation
> [ 25.407433]
> [ 25.414214] 4 locks held by login/704:
> [ 25.417955] #0: (((&ndev->rs_timer))){+.-...}, at: [<c002f8a8>] call_timer_fn+0x0/0xe4
> [ 25.426076] #1: (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
> [ 25.434018] #2: (rcu_read_lock){.+.+..}, at: [<c048e104>] ndisc_send_skb+0x58/0x3dc
> [ 25.441876] #3: (rcu_read_lock){.+.+..}, at: [<c049bdc8>] ipv6_chk_mcast_addr+0x0/0x200
> [ 25.450079]
> [ 25.450079] stack backtrace:
> [ 25.454436] CPU: 0 PID: 704 Comm: login Not tainted 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17
> [ 25.463674] [<c0016958>] (unwind_backtrace+0x0/0x138) from [<c00125fc>] (show_stack+0x10/0x14)
> [ 25.472297] [<c00125fc>] (show_stack+0x10/0x14) from [<c0567808>] (dump_stack+0x80/0xc4)
> [ 25.480394] [<c0567808>] (dump_stack+0x80/0xc4) from [<c0074af8>] (validate_chain.isra.24+0x74c/0x8c8)
> [ 25.489696] [<c0074af8>] (validate_chain.isra.24+0x74c/0x8c8) from [<c00757b0>] (__lock_acquire+0x3bc/0xa80)
> [ 25.499518] [<c00757b0>] (__lock_acquire+0x3bc/0xa80) from [<c007632c>] (lock_acquire+0x60/0x74)
> [ 25.508305] [<c007632c>] (lock_acquire+0x60/0x74) from [<c056e998>] (_raw_read_lock_bh+0x4c/0x5c)
> [ 25.517175] [<c056e998>] (_raw_read_lock_bh+0x4c/0x5c) from [<c049be24>] (ipv6_chk_mcast_addr+0x5c/0x200)
> [ 25.526740] [<c049be24>] (ipv6_chk_mcast_addr+0x5c/0x200) from [<c04799ac>] (ip6_finish_output2+0x298/0x62c)
> [ 25.536563] [<c04799ac>] (ip6_finish_output2+0x298/0x62c) from [<c048e2b0>] (ndisc_send_skb+0x204/0x3dc)
> [ 25.546037] [<c048e2b0>] (ndisc_send_skb+0x204/0x3dc) from [<c0480fa4>] (addrconf_rs_timer+0x108/0x134)
> [ 25.555425] [<c0480fa4>] (addrconf_rs_timer+0x108/0x134) from [<c002f914>] (call_timer_fn+0x6c/0xe4)
> [ 25.564553] [<c002f914>] (call_timer_fn+0x6c/0xe4) from [<c0030220>] (run_timer_softirq+0x180/0x22c)
> [ 25.573691] [<c0030220>] (run_timer_softirq+0x180/0x22c) from [<c002a210>] (__do_softirq+0x100/0x1fc)
> [ 25.582905] [<c002a210>] (__do_softirq+0x100/0x1fc) from [<c002a6d8>] (irq_exit+0xa8/0xf4)
> [ 25.591166] [<c002a6d8>] (irq_exit+0xa8/0xf4) from [<c000f9b8>] (handle_IRQ+0x3c/0x94)
> [ 25.599078] [<c000f9b8>] (handle_IRQ+0x3c/0x94) from [<c0008700>] (gic_handle_irq+0x28/0x5c)
> [ 25.607511] [<c0008700>] (gic_handle_irq+0x28/0x5c) from [<c00132fc>] (__irq_usr+0x3c/0x60)
> [ 25.615855] Exception stack(0xee0d9fb0 to 0xee0d9ff8)
> [ 25.620901] 9fa0: 5c026e4c c9ce71d5 538e8b11 f2a43512
> [ 25.629072] 9fc0: 49f97e42 7a179ff1 9c755700 9b933273 8b7a2401 6d6222ee 7b8908fd 1df2f704
> [ 25.637243] 9fe0: 2664e738 bec3ef10 b6d440e9 b6d43d58 900f0030 ffffffff
^ permalink raw reply
* Re: [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
From: Hannes Frederic Sowa @ 2013-09-03 18:09 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-4-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:33AM +0200, Daniel Borkmann wrote:
> Get rid of MLDV2_MRC and use our new macros for mantisse and
> exponent to calculate Maximum Response Delay out of the Maximum
> Response Code.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ 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