* [net-next PATCH] net: codel: Avoid undefined behavior from signed overflow
From: Jesper Dangaard Brouer @ 2013-10-30 17:23 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Paul E. McKenney, Dave Taht, Jesper Dangaard Brouer
From: Jesper Dangaard Brouer <netoptimizer@brouer.com>
As described in commit 5a581b367 (jiffies: Avoid undefined
behavior from signed overflow), according to the C standard
3.4.3p3, overflow of a signed integer results in undefined
behavior.
To fix this, do as the above commit, and do an unsigned
subtraction, and interpreting the result as a signed
two's-complement number. This is based on the theory from
RFC 1982 and is nicely described in wikipedia here:
https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
A side-note, I have seen practical issues with the previous logic
when dealing with 16-bit, on a 64-bit machine (gcc version
4.4.5). This were 32-bit, which I have not observed issues with.
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com>
---
include/net/codel.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/net/codel.h b/include/net/codel.h
index 389cf62..700fcdf 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -72,10 +72,10 @@ static inline codel_time_t codel_get_time(void)
return ns >> CODEL_SHIFT;
}
-#define codel_time_after(a, b) ((s32)(a) - (s32)(b) > 0)
-#define codel_time_after_eq(a, b) ((s32)(a) - (s32)(b) >= 0)
-#define codel_time_before(a, b) ((s32)(a) - (s32)(b) < 0)
-#define codel_time_before_eq(a, b) ((s32)(a) - (s32)(b) <= 0)
+#define codel_time_after(a, b) ((s32)((a) - (b)) > 0)
+#define codel_time_after_eq(a, b) ((s32)((a) - (b)) >= 0)
+#define codel_time_before(a, b) ((s32)((a) - (b)) < 0)
+#define codel_time_before_eq(a, b) ((s32)((a) - (b)) <= 0)
/* Qdiscs using codel plugin must use codel_skb_cb in their own cb[] */
struct codel_skb_cb {
^ permalink raw reply related
* [patch v2] libertas: potential oops in debugfs
From: Dan Carpenter @ 2013-10-30 17:12 UTC (permalink / raw)
To: John W. Linville
Cc: libertas-dev, linux-wireless, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <20131025144452.GA28451@ngolde.de>
If we do a zero size allocation then it will oops. Also we can't be
sure the user passes us a NUL terminated string so I've added a
terminator.
This code can only be triggered by root.
Reported-by: Nico Golde <nico@ngolde.de>
Reported-by: Fabian Yamaguchi <fabs@goesec.de>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
index 668dd27..1917348 100644
--- a/drivers/net/wireless/libertas/debugfs.c
+++ b/drivers/net/wireless/libertas/debugfs.c
@@ -913,7 +913,10 @@ static ssize_t lbs_debugfs_write(struct file *f, const char __user *buf,
char *p2;
struct debug_data *d = f->private_data;
- pdata = kmalloc(cnt, GFP_KERNEL);
+ if (cnt == 0)
+ return 0;
+
+ pdata = kmalloc(cnt + 1, GFP_KERNEL);
if (pdata == NULL)
return 0;
@@ -922,6 +925,7 @@ static ssize_t lbs_debugfs_write(struct file *f, const char __user *buf,
kfree(pdata);
return 0;
}
+ pdata[cnt] = '\0';
p0 = pdata;
for (i = 0; i < num_of_items; i++) {
^ permalink raw reply related
* Re: [PATCH] route: Exporting ip6_route_add() so that Bluetooth 6LoWPAN can use it
From: David Miller @ 2013-10-30 17:05 UTC (permalink / raw)
To: jukka.rissanen; +Cc: netdev
In-Reply-To: <1383124637-15533-1-git-send-email-jukka.rissanen@linux.intel.com>
From: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Date: Wed, 30 Oct 2013 11:17:17 +0200
> Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Sorry, no.
There is no reason that external subsystems need to add ipv6 routes
using direct calls to these functions.
^ permalink raw reply
* Re: [PATCH net-next 1/5] lib: crc32: clean up spacing in test cases
From: Joe Perches @ 2013-10-30 15:14 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Laight, davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <52711481.8030003@redhat.com>
On Wed, 2013-10-30 at 15:15 +0100, Daniel Borkmann wrote:
> On 10/30/2013 03:10 PM, David Laight wrote:
> >>> + {0x674bf11d, 0x00000038, 0x00000542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
> >> these could be
> >> + {0x674bf11d, 0x38, 0x542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
> > Or even:
> > #define X(a, b, c, d, e, f) {0x##a, 0x##b, 0x##c, 0x##d, 0x##e. 0x##f}
> > X(674bf11d, 38, 542, 0af6d466, d8b6e4c1, f6e93d6c),
> > ...
> > #undef X
>
> Sure, sounds good to me. We could do that as a follow-up.
I personally don't care for that sort of token-pasting macro
but, <shrug> if you want, I've no real objection either.
^ permalink raw reply
* [PATCH net-next] net: extend net_device allocation to vmalloc()
From: Eric Dumazet @ 2013-10-30 15:01 UTC (permalink / raw)
To: Joby Poriyath, David Miller
Cc: netdev, wei.liu2, ian.campbell, xen-devel, andrew.bennieston,
david.vrabel, malcolm.crossley
In-Reply-To: <1383141242.4857.36.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
Joby Poriyath provided a xen-netback patch to reduce the size of
xenvif structure as some netdev allocation could fail under
memory pressure/fragmentation.
This patch is handling the problem at the core level, allowing
any netdev structures to use vmalloc() if kmalloc() failed.
As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT
to kzalloc() flags to do this fallback only when really needed.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Joby Poriyath <joby.poriyath@citrix.com>
---
Documentation/networking/netdevices.txt | 7 ++++---
include/linux/netdevice.h | 1 +
net/core/dev.c | 22 +++++++++++++++++-----
net/core/net-sysfs.c | 2 +-
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/Documentation/networking/netdevices.txt b/Documentation/networking/netdevices.txt
index c7ecc70..df7cd19 100644
--- a/Documentation/networking/netdevices.txt
+++ b/Documentation/networking/netdevices.txt
@@ -10,9 +10,10 @@ network devices.
struct net_device allocation rules
==================================
Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc. If device has registered successfully,
-it will be freed on last use by free_netdev. This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with kmalloc() or vmalloc(). If device has registered
+successfully, it will be freed on last use by free_netdev.
+This is required to handle the pathologic case cleanly
+(example: rmmod mydriver </sys/class/net/myeth/mtu )
There are routines in net_init.c to handle the common cases of
alloc_etherdev, alloc_netdev. These reserve extra space for driver
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cb1d918..e6353ca 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1800,6 +1800,7 @@ static inline void unregister_netdevice(struct net_device *dev)
int netdev_refcnt_read(const struct net_device *dev);
void free_netdev(struct net_device *dev);
+void netdev_freemem(struct net_device *dev);
void synchronize_net(void);
int init_dummy_netdev(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 0054c8c..0e61365 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6196,6 +6196,16 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
+void netdev_freemem(struct net_device *dev)
+{
+ char *addr = (char *)dev - dev->padded;
+
+ if (is_vmalloc_addr(addr))
+ vfree(addr);
+ else
+ kfree(addr);
+}
+
/**
* alloc_netdev_mqs - allocate network device
* @sizeof_priv: size of private data to allocate space for
@@ -6239,7 +6249,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
/* ensure 32-byte alignment of whole construct */
alloc_size += NETDEV_ALIGN - 1;
- p = kzalloc(alloc_size, GFP_KERNEL);
+ p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+ if (!p)
+ p = vzalloc(alloc_size);
if (!p)
return NULL;
@@ -6248,7 +6260,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
dev->pcpu_refcnt = alloc_percpu(int);
if (!dev->pcpu_refcnt)
- goto free_p;
+ goto free_dev;
if (dev_addr_init(dev))
goto free_pcpu;
@@ -6301,8 +6313,8 @@ free_pcpu:
kfree(dev->_rx);
#endif
-free_p:
- kfree(p);
+free_dev:
+ netdev_freemem(dev);
return NULL;
}
EXPORT_SYMBOL(alloc_netdev_mqs);
@@ -6339,7 +6351,7 @@ void free_netdev(struct net_device *dev)
/* Compatibility with error handling in drivers */
if (dev->reg_state == NETREG_UNINITIALIZED) {
- kfree((char *)dev - dev->padded);
+ netdev_freemem(dev);
return;
}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index d954b56..d03f2c9 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1263,7 +1263,7 @@ static void netdev_release(struct device *d)
BUG_ON(dev->reg_state != NETREG_RELEASED);
kfree(dev->ifalias);
- kfree((char *)dev - dev->padded);
+ netdev_freemem(dev);
}
static const void *net_namespace(struct device *d)
^ permalink raw reply related
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Neil Horman @ 2013-10-30 14:52 UTC (permalink / raw)
To: Doug Ledford
Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev, David Laight
In-Reply-To: <52710B09.6090302@redhat.com>
On Wed, Oct 30, 2013 at 09:35:05AM -0400, Doug Ledford wrote:
> On 10/30/2013 07:02 AM, Neil Horman wrote:
>
> >That does makes sense, but it then begs the question, whats the advantage of
> >having multiple alu's at all?
>
> There's lots of ALU operations that don't operate on the flags or
> other entities that can be run in parallel.
>
> >If they're just going to serialize on the
> >updating of the condition register, there doesn't seem to be much advantage in
> >having multiple alu's at all, especially if a common use case (parallelizing an
> >operation on a large linear dataset) resulted in lower performance.
> >
> >/me wonders if rearranging the instructions into this order:
> >adcq 0*8(src), res1
> >adcq 1*8(src), res2
> >adcq 2*8(src), res1
> >
> >would prevent pipeline stalls. That would be interesting data, and (I think)
> >support your theory, Doug. I'll give that a try
>
> Just to avoid spending too much time on various combinations, here
> are the methods I've tried:
>
> Original code
> 2 chains doing interleaved memory accesses
> 2 chains doing serial memory accesses (as above)
> 4 chains doing serial memory accesses
> 4 chains using 32bit values in 64bit registers so you can always use
> add instead of adc and never need the carry flag
>
> And I've done all of the above with simple prefetch and smart prefetch.
>
Yup, I just tried the 2 chains doing interleaved access and came up with the
same results for both prefetch cases.
> In all cases, the result is basically that the add method doesn't
> matter much in the grand scheme of things, but the prefetch does,
> and smart prefetch always beat simple prefetch.
>
> My simple prefetch was to just go into the main while() loop for the
> csum operation and always prefetch 5*64 into the future.
>
> My smart prefetch looks like this:
>
> static inline void prefetch_line(unsigned long *cur_line,
> unsigned long *end_line,
> size_t size)
> {
> size_t fetched = 0;
>
> while (*cur_line <= *end_line && fetched < size) {
> prefetch((void *)*cur_line);
> *cur_line += cache_line_size();
> fetched += cache_line_size();
> }
> }
>
I've done this too, but I've come up with results that are very close to simple
prefetch.
> I was going to tinker today and tomorrow with this function once I
> get a toolchain that will compile it (I reinstalled all my rhel6
> hosts as f20 and I'm hoping that does the trick, if not I need to do
> more work):
>
> #define ADCXQ_64 \
> asm("xorq %[res1],%[res1]\n\t" \
> "adcxq 0*8(%[src]),%[res1]\n\t" \
> "adoxq 1*8(%[src]),%[res2]\n\t" \
> "adcxq 2*8(%[src]),%[res1]\n\t" \
> "adoxq 3*8(%[src]),%[res2]\n\t" \
> "adcxq 4*8(%[src]),%[res1]\n\t" \
> "adoxq 5*8(%[src]),%[res2]\n\t" \
> "adcxq 6*8(%[src]),%[res1]\n\t" \
> "adoxq 7*8(%[src]),%[res2]\n\t" \
> "adcxq %[zero],%[res1]\n\t" \
> "adoxq %[zero],%[res2]\n\t" \
> : [res1] "=r" (result1), \
> [res2] "=r" (result2) \
> : [src] "r" (buff), [zero] "r" (zero), \
> "[res1]" (result1), "[res2]" (result2))
>
I've tried using this method also (HPA suggested it early in the thread, but its
not going to be usefull for awhile. The compiler supports it already, but
theres not hardware available with support for these instructions yet (at least
not that I have available).
Neil
^ permalink raw reply
* Re: [patch net-next RFC] netfilter: ip6_tables: use reasm skb for matching
From: Florian Westphal @ 2013-10-30 14:44 UTC (permalink / raw)
To: Jiri Pirko
Cc: Florian Westphal, netdev, davem, pablo, netfilter-devel, yoshfuji,
kadlec, kaber, mleitner
In-Reply-To: <20131030141354.GB1456@minipsycho.orion>
Jiri Pirko <jiri@resnulli.us> wrote:
> >This is a bit backwards, I think.
> >- We gather frags
> >- Then we invoke ip6t_do_table for each individual fragment
> >
> >So basically your patch is equivalent to
> >for_each_frag( )
> > ip6t_do_table(reassembled_skb)
> >
> >Which makes no sense to me - why traverse the ruleset n times with the same
> >packet?
>
> Because each fragment need to be pushed through separately.
Why? AFAIU we only need to ensure that (in forwarding case) we
send out the original fragments instead of the reassembled packet.
> What different approach would you suggest?
I am sure that current behaviour is intentional, so I'd first like to
understand WHY this was implemented this way.
Also, this would change very long standing behaviour so one might argue that
this is a no-go anyway.
What is the exact problem that this is supposed to solve?
^ permalink raw reply
* Re: [PATCH net-next 0/5] SCTP fix/updates
From: Vlad Yasevich @ 2013-10-30 14:29 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev, linux-sctp
In-Reply-To: <1383130252-1515-1-git-send-email-dborkman@redhat.com>
On 10/30/2013 06:50 AM, Daniel Borkmann wrote:
> Please see patch 5 for the main description/motivation, the rest just
> brings in the needed functionality for that. Although this is actually
> a fix, I've based it against net-next as some additional work for
> fixing it was needed.
>
> Thanks!
>
> Daniel Borkmann (5):
> lib: crc32: clean up spacing in test cases
> lib: crc32: add functionality to combine two crc32{,c}s in GF(2)
> lib: crc32: add test cases for crc32{,c}_combine routines
> net: skb_checksum: allow custom update/combine for walking skb
> net: sctp: fix and consolidate SCTP checksumming code
>
> include/linux/crc32.h | 40 ++++
> include/linux/skbuff.h | 13 +-
> include/net/checksum.h | 6 +
> include/net/sctp/checksum.h | 56 ++----
> lib/crc32.c | 453 +++++++++++++++++++++++++-------------------
> net/core/skbuff.c | 31 ++-
> net/sctp/output.c | 9 +-
> 7 files changed, 350 insertions(+), 258 deletions(-)
>
Series
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
^ permalink raw reply
* Re: [PATCH net-next 5/5] net: sctp: fix and consolidate SCTP checksumming code
From: Vlad Yasevich @ 2013-10-30 14:29 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev, linux-sctp
In-Reply-To: <1383130252-1515-6-git-send-email-dborkman@redhat.com>
On 10/30/2013 06:50 AM, Daniel Borkmann wrote:
> This fixes an outstanding bug found through IPVS, where SCTP packets
> with skb->data_len > 0 (non-linearized) and empty frag_list, but data
> accumulated in frags[] member, are forwarded with incorrect checksum
> letting SCTP initial handshake fail on some systems. Linearizing each
> SCTP skb in IPVS to prevent that would not be a good solution as
> this leads to an additional and unnecessary performance penalty on
> the load-balancer itself for no good reason (as we actually only want
> to update the checksum, and can do that in a different/better way
> presented here).
>
> The actual problem is elsewhere, namely, that SCTP's checksumming
> in sctp_compute_cksum() does not take frags[] into account like
> skb_checksum() does. So while we are fixing this up, we better reuse
> the existing code that we have anyway in __skb_checksum() and use it
> for walking through the data doing checksumming. This will not only
> fix this issue, but also consolidates some SCTP code with core
> sk_buff code, bringing it closer together and removing respectively
> avoiding reimplementation of skb_checksum() for no good reason.
>
> As crc32c() can use hardware implementation within the crypto layer,
> we leave that intact (it wraps around / falls back to e.g. slice-by-8
> algorithm in __crc32c_le() otherwise); plus use the __crc32c_le_combine()
> combinator for crc32c blocks.
>
> Also, we remove all other SCTP checksumming code, so that we only
> have to use sctp_compute_cksum() from now on; for doing that, we need
> to transform SCTP checkumming in output path slightly, and can leave
> the rest intact.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Daniel
Here is a follow-on idea that might help even more.
What if we put a pointer to skb_checksum_ops() in the skb
somewhere (I was thinking of skb_shinfo). Then
skb_checksum can simply use the data from there. This would
allow us to get rid of all the special cases in SCTP that do
checksumming. We can just set it to partial, set up the right
fields and let HW or SW always do the right thing.
-vlad
> ---
> include/net/sctp/checksum.h | 56 +++++++++++++++------------------------------
> net/sctp/output.c | 9 +-------
> 2 files changed, 20 insertions(+), 45 deletions(-)
>
> diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> index 259924d..6bd44fe 100644
> --- a/include/net/sctp/checksum.h
> +++ b/include/net/sctp/checksum.h
> @@ -42,56 +42,38 @@
> #include <linux/types.h>
> #include <net/sctp/sctp.h>
> #include <linux/crc32c.h>
> +#include <linux/crc32.h>
>
> -static inline __u32 sctp_crc32c(__u32 crc, u8 *buffer, u16 length)
> +static inline __wsum sctp_csum_update(const void *buff, int len, __wsum sum)
> {
> - return crc32c(crc, buffer, length);
> -}
> -
> -static inline __u32 sctp_start_cksum(__u8 *buffer, __u16 length)
> -{
> - __u32 crc = ~(__u32)0;
> - __u8 zero[sizeof(__u32)] = {0};
> -
> - /* Optimize this routine to be SCTP specific, knowing how
> - * to skip the checksum field of the SCTP header.
> + /* This uses the crypto implementation of crc32c, which is either
> + * implemented w/ hardware support or resolves to __crc32c_le().
> */
> -
> - /* Calculate CRC up to the checksum. */
> - crc = sctp_crc32c(crc, buffer, sizeof(struct sctphdr) - sizeof(__u32));
> -
> - /* Skip checksum field of the header. */
> - crc = sctp_crc32c(crc, zero, sizeof(__u32));
> -
> - /* Calculate the rest of the CRC. */
> - crc = sctp_crc32c(crc, &buffer[sizeof(struct sctphdr)],
> - length - sizeof(struct sctphdr));
> - return crc;
> -}
> -
> -static inline __u32 sctp_update_cksum(__u8 *buffer, __u16 length, __u32 crc32)
> -{
> - return sctp_crc32c(crc32, buffer, length);
> + return crc32c(sum, buff, len);
> }
>
> -static inline __le32 sctp_end_cksum(__u32 crc32)
> +static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> + int offset, int len)
> {
> - return cpu_to_le32(~crc32);
> + return __crc32c_le_combine(csum, csum2, len);
> }
>
> -/* Calculate the CRC32C checksum of an SCTP packet. */
> static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> unsigned int offset)
> {
> - const struct sk_buff *iter;
> + struct sctphdr *sh = sctp_hdr(skb);
> + __le32 ret, old = sh->checksum;
> + const struct skb_checksum_ops ops = {
> + .update = sctp_csum_update,
> + .combine = sctp_csum_combine,
> + };
>
> - __u32 crc32 = sctp_start_cksum(skb->data + offset,
> - skb_headlen(skb) - offset);
> - skb_walk_frags(skb, iter)
> - crc32 = sctp_update_cksum((__u8 *) iter->data,
> - skb_headlen(iter), crc32);
> + sh->checksum = 0;
> + ret = cpu_to_le32(~__skb_checksum(skb, offset, skb->len - offset,
> + ~(__u32)0, &ops));
> + sh->checksum = old;
>
> - return sctp_end_cksum(crc32);
> + return ret;
> }
>
> #endif /* __sctp_checksum_h__ */
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 3191373..e650978 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -390,7 +390,6 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> __u8 has_data = 0;
> struct dst_entry *dst = tp->dst;
> unsigned char *auth = NULL; /* pointer to auth in skb data */
> - __u32 cksum_buf_len = sizeof(struct sctphdr);
>
> pr_debug("%s: packet:%p\n", __func__, packet);
>
> @@ -493,7 +492,6 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> if (chunk == packet->auth)
> auth = skb_tail_pointer(nskb);
>
> - cksum_buf_len += chunk->skb->len;
> memcpy(skb_put(nskb, chunk->skb->len),
> chunk->skb->data, chunk->skb->len);
>
> @@ -538,12 +536,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> if (!sctp_checksum_disable) {
> if (!(dst->dev->features & NETIF_F_SCTP_CSUM) ||
> (dst_xfrm(dst) != NULL) || packet->ipfragok) {
> - __u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
> -
> - /* 3) Put the resultant value into the checksum field in the
> - * common header, and leave the rest of the bits unchanged.
> - */
> - sh->checksum = sctp_end_cksum(crc32);
> + sh->checksum = sctp_compute_cksum(nskb, 0);
> } else {
> /* no need to seed pseudo checksum for SCTP */
> nskb->ip_summed = CHECKSUM_PARTIAL;
>
^ permalink raw reply
* Re: [PATCH 1/4 net-next] net: phy: add Generic Netlink Ethernet switch configuration API
From: Maxime Bizon @ 2013-10-30 14:28 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Felix Fietkau, Florian Fainelli, Neil Horman, John Fastabend,
netdev, David Miller, Sascha Hauer, John Crispin, Jonas Gorski,
Gary Thomas, Vlad Yasevich, Stephen Hemminger
In-Reply-To: <5270F282.6000708@mojatatu.com>
On Wed, 2013-10-30 at 07:50 -0400, Jamal Hadi Salim wrote:
> The important part is all the APIs stay consistent. I can use
> same netlink calls. ifconfig works.
> iproute2 works. People have written books on this stuff - we dont
these books usually start by telling people to assign IP address to
interfaces, not applicable here.
> If i can get stats by doing ifconfig - that should provide illusion that
> the netdevice is sending/receiving packets.
4 separated netdevices looks like 4 ethernet segments to me, and nothing
will prevent me from setting a different ip network on each device.
ENOTSUPP cannot be returned by ndo_start_xmit, the ability for a
netdevice to be able to receive/send packet from host is IMO
fundamental.
> This is a good arguement.
> Can we hear a little more about this?
see this kind of old threads:
http://rt2x00.serialmonkey.com/phpBB/viewtopic.php?f=5&t=4378
http://www.linuxquestions.org/questions/linux-networking-3/what-is-wmaster0-728708/
http://forums.debian.net/viewtopic.php?p=219440
> I think that would be a reasonable thing to do if it becomes
> necessary.
with rough naming:
- struct netdevice
- struct netdev_queue
- struct network_port (something to call ethtool on)
- struct bridge_dev (something you create/destroy vlan on, control FDB)
- struct bridge_port (something you set path cost on, ...)
- struct sw_bridge_dev (netdevice + underlying bridge_dev)
- struct sw_bridge_port (netdevice + underlying bridge_port)
old netdevice => (netdevice + netdev_queue * x + network_port)
ethtool works on netdevice or network_port
brctl addbr/addif creates sw_bridge_dev/sw_bridge_port, other commands
work on bridge_dev/bridge_port
drivers can register bridge_dev / bridge_port / network_port
simple case of a system with single ethernet mac & directly attached 4
ports switch:
netdevice: eth0
bridge_dev: hwbr0
bridge_port: hwbr0p0, hwbr0p1, hwbr0p2, hwbr0p3
network ports: eth0np0, hwbr0np0, hwbr0np1, hwbr0np2, hwbr0np3
ifconfig, ip link show only eth0
brctl show hwbr0
ethtool works on eth0 or eth0p0, hwbr0npX
--
Maxime
^ permalink raw reply
* Re: [PATCH net-next 1/5] lib: crc32: clean up spacing in test cases
From: Daniel Borkmann @ 2013-10-30 14:15 UTC (permalink / raw)
To: David Laight; +Cc: Joe Perches, davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73C8@saturn3.aculab.com>
On 10/30/2013 03:10 PM, David Laight wrote:
>>> + {0x674bf11d, 0x00000038, 0x00000542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
>>
>> these could be
>>
>> + {0x674bf11d, 0x38, 0x542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
>
> Or even:
> #define X(a, b, c, d, e, f) {0x##a, 0x##b, 0x##c, 0x##d, 0x##e. 0x##f}
> X(674bf11d, 38, 542, 0af6d466, d8b6e4c1, f6e93d6c),
> ...
> #undef X
Sure, sounds good to me. We could do that as a follow-up.
> David
>
>
>
^ permalink raw reply
* Re: [patch net-next RFC] netfilter: ip6_tables: use reasm skb for matching
From: Jiri Pirko @ 2013-10-30 14:13 UTC (permalink / raw)
To: Florian Westphal
Cc: netdev, davem, pablo, netfilter-devel, yoshfuji, kadlec, kaber,
mleitner
In-Reply-To: <20131030134100.GD16615@breakpoint.cc>
Wed, Oct 30, 2013 at 02:41:00PM CET, fw@strlen.de wrote:
>Jiri Pirko <jiri@resnulli.us> wrote:
>> Currently, when ipv6 fragment goes through the netfilter, match
>> functions are called on them directly. This might cause match function
>> to fail. So benefit from the fact that nf_defrag_ipv6 constructs
>> reassembled skb for us and use this reassembled skb for matching.
>>
>> This patch fixes for example following situation:
>> On HOSTA do:
>> ip6tables -I INPUT -p icmpv6 -j DROP
>> ip6tables -I INPUT -p icmpv6 -m icmp6 --icmpv6-type 128 -j ACCEPT
>>
>> and on HOSTB you do:
>> ping6 HOSTA -s2000 (MTU is 1500)
>>
>> Incoming echo requests will be filtered out on HOSTA. This issue does
>> not occur with smaller packets than MTU (where fragmentation does not happen).
>
>[..]
>> diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
>> index 44400c2..5421beb0 100644
>> --- a/net/ipv6/netfilter/ip6_tables.c
>> +++ b/net/ipv6/netfilter/ip6_tables.c
>> @@ -328,6 +328,7 @@ ip6t_do_table(struct sk_buff *skb,
>> const struct xt_table_info *private;
>> struct xt_action_param acpar;
>> unsigned int addend;
>> + struct sk_buff *reasm = skb->nfct_reasm ? skb->nfct_reasm : skb;
>>
>> /* Initialization */
>> indev = in ? in->name : nulldevname;
>> @@ -363,7 +364,7 @@ ip6t_do_table(struct sk_buff *skb,
>>
>> IP_NF_ASSERT(e);
>> acpar.thoff = 0;
>> - if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
>> + if (!ip6_packet_match(reasm, indev, outdev, &e->ipv6,
>> &acpar.thoff, &acpar.fragoff, &acpar.hotdrop)) {
>
>[..]
>
>This is a bit backwards, I think.
>- We gather frags
>- Then we invoke ip6t_do_table for each individual fragment
>
>So basically your patch is equivalent to
>for_each_frag( )
> ip6t_do_table(reassembled_skb)
>
>Which makes no sense to me - why traverse the ruleset n times with the same
>packet?
Because each fragment need to be pushed through separately.
What different approach would you suggest?
Thanks
Jiri
^ permalink raw reply
* RE: [PATCH net-next 1/5] lib: crc32: clean up spacing in test cases
From: David Laight @ 2013-10-30 14:10 UTC (permalink / raw)
To: Joe Perches, Daniel Borkmann; +Cc: davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <1383141418.12439.76.camel@joe-AO722>
> > + {0x674bf11d, 0x00000038, 0x00000542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
>
> these could be
>
> + {0x674bf11d, 0x38, 0x542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
Or even:
#define X(a, b, c, d, e, f) {0x##a, 0x##b, 0x##c, 0x##d, 0x##e. 0x##f}
X(674bf11d, 38, 542, 0af6d466, d8b6e4c1, f6e93d6c),
...
#undef X
David
^ permalink raw reply
* RE: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: David Laight @ 2013-10-30 14:04 UTC (permalink / raw)
To: Doug Ledford, Neil Horman; +Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <52710B09.6090302@redhat.com>
...
> and then I also wanted to try using both xmm and ymm registers and doing
> 64bit adds with 32bit numbers across multiple xmm/ymm registers as that
> should parallel nicely. David, you mentioned you've tried this, how did
> your experiment turn out and what was your method? I was planning on
> doing regular full size loads into one xmm/ymm register, then using
> pshufd/vshufd to move the data into two different registers, then
> summing into a fourth register, and possible running two of those
> pipelines in parallel.
It was a long time ago, and IIRC the code was just SSE so the
register length just wasn't going to give the required benefit.
I know I wrote the code, but I can't even remember whether I
actually got it working!
With the longer AVX words it might make enough difference.
Of course, this assumes that you have the fpu registers
available. If you have to do a fpu context switch it will
be a lot slower.
About the same time I did manage to an open coded copy
loop to run as fast as 'rep movs' - and without any unrolling
or any prefetch instructions.
Thinking about AVX you should be able to do (without looking up the
actual mnemonics):
load
add 32bit chunks to sum
compare sum with read value (equiv of carry)
add/subtract compare result (0 or ~0) to a carry-sum register
That is 4 instructions for 256 bits, so you can aim for 4 clocks.
You'd need to check the cpu book to see if any of those can
be scheduled at the same time (if not dependant).
(and also whether there is any result delay - don't think so.)
I'd try running two copies of the above - probably skewed so that
the memory accesses are separated, do the memory read for the
next iteration, and use the 3rd instruction unit for loop control.
David
^ permalink raw reply
* Re: [PATCH ] net_sched: actions - Add default lookup
From: Eric Dumazet @ 2013-10-30 14:00 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, netdev@vger.kernel.org, Eric W. Biederman,
Alexander Duyck
In-Reply-To: <5270ECB5.2030601@mojatatu.com>
On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
> Attached. Tested with simple action.
>
> cheers,
> jamal
Why not setting .lookup to tcf_hash_search
in the few actions not already doing that ?
This would be more consistent.
# git grep -n tcf_hash_search
include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
net/sched/act_csum.c:588: .lookup = tcf_hash_search,
net/sched/act_gact.c:209: .lookup = tcf_hash_search,
net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
net/sched/act_nat.c:311: .lookup = tcf_hash_search,
net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
net/sched/act_police.c:410: .lookup = tcf_hash_search,
^ permalink raw reply
* Re: [PATCH net-next 1/5] lib: crc32: clean up spacing in test cases
From: Joe Perches @ 2013-10-30 13:56 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp, linux-kernel
In-Reply-To: <1383130252-1515-2-git-send-email-dborkman@redhat.com>
On Wed, 2013-10-30 at 11:50 +0100, Daniel Borkmann wrote:
> This is nothing more but a whitepace cleanup, as 80 chars is not a
> hard but soft limit, and otherwise makes the test cases arrary really
> look ugly. So fix it up.
That does look nicer.
Another option might be to take the repetitive
6 leading 0's out of column 2 and the repetitive
5 leading 0's out of column 3.
> diff --git a/lib/crc32.c b/lib/crc32.c
[]
> @@ -795,206 +795,106 @@ static struct crc_test {
> u32 crc32c_le; /* expected crc32c_le result */
> } test[] =
> {
> - {0x674bf11d, 0x00000038, 0x00000542, 0x0af6d466, 0xd8b6e4c1,
> - 0xf6e93d6c},
> - {0x35c672c6, 0x0000003a, 0x000001aa, 0xc6d3dfba, 0x28aaf3ad,
> - 0x0fe92aca},
[etc...]
> + {0x674bf11d, 0x00000038, 0x00000542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
> + {0x35c672c6, 0x0000003a, 0x000001aa, 0xc6d3dfba, 0x28aaf3ad, 0x0fe92aca},
these could be
+ {0x674bf11d, 0x38, 0x542, 0x0af6d466, 0xd8b6e4c1, 0xf6e93d6c},
+ {0x35c672c6, 0x3a, 0x1aa, 0xc6d3dfba, 0x28aaf3ad, 0x0fe92aca},
etc...
^ permalink raw reply
* Re: [PATCH net-next] xen-netback: allocate xenvif arrays using vzalloc.
From: Eric Dumazet @ 2013-10-30 13:54 UTC (permalink / raw)
To: Joby Poriyath
Cc: wei.liu2, ian.campbell, netdev, xen-devel, david.vrabel,
malcolm.crossley, andrew.bennieston
In-Reply-To: <20131030103958.GB3261@citrix.com>
On Wed, 2013-10-30 at 10:39 +0000, Joby Poriyath wrote:
> The net_device allocation rule {linux/Documentation/networking/netdevices.txt} states
> that net_device struct must be allocated using kmalloc.
>
> Is this safe to do?
As long as the freeing path is aware of the possibility the memory is
either allocated with vmalloc() or kmalloc(), we are safe.
^ permalink raw reply
* Re: [patch net-next RFC] netfilter: ip6_tables: use reasm skb for matching
From: Florian Westphal @ 2013-10-30 13:41 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, pablo, netfilter-devel, yoshfuji, kadlec, kaber,
mleitner
In-Reply-To: <1383130201-6198-1-git-send-email-jiri@resnulli.us>
Jiri Pirko <jiri@resnulli.us> wrote:
> Currently, when ipv6 fragment goes through the netfilter, match
> functions are called on them directly. This might cause match function
> to fail. So benefit from the fact that nf_defrag_ipv6 constructs
> reassembled skb for us and use this reassembled skb for matching.
>
> This patch fixes for example following situation:
> On HOSTA do:
> ip6tables -I INPUT -p icmpv6 -j DROP
> ip6tables -I INPUT -p icmpv6 -m icmp6 --icmpv6-type 128 -j ACCEPT
>
> and on HOSTB you do:
> ping6 HOSTA -s2000 (MTU is 1500)
>
> Incoming echo requests will be filtered out on HOSTA. This issue does
> not occur with smaller packets than MTU (where fragmentation does not happen).
[..]
> diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> index 44400c2..5421beb0 100644
> --- a/net/ipv6/netfilter/ip6_tables.c
> +++ b/net/ipv6/netfilter/ip6_tables.c
> @@ -328,6 +328,7 @@ ip6t_do_table(struct sk_buff *skb,
> const struct xt_table_info *private;
> struct xt_action_param acpar;
> unsigned int addend;
> + struct sk_buff *reasm = skb->nfct_reasm ? skb->nfct_reasm : skb;
>
> /* Initialization */
> indev = in ? in->name : nulldevname;
> @@ -363,7 +364,7 @@ ip6t_do_table(struct sk_buff *skb,
>
> IP_NF_ASSERT(e);
> acpar.thoff = 0;
> - if (!ip6_packet_match(skb, indev, outdev, &e->ipv6,
> + if (!ip6_packet_match(reasm, indev, outdev, &e->ipv6,
> &acpar.thoff, &acpar.fragoff, &acpar.hotdrop)) {
[..]
This is a bit backwards, I think.
- We gather frags
- Then we invoke ip6t_do_table for each individual fragment
So basically your patch is equivalent to
for_each_frag( )
ip6t_do_table(reassembled_skb)
Which makes no sense to me - why traverse the ruleset n times with the same
packet?
^ permalink raw reply
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Doug Ledford @ 2013-10-30 13:35 UTC (permalink / raw)
To: Neil Horman; +Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev, David Laight
In-Reply-To: <20131030110214.GA10220@localhost.localdomain>
On 10/30/2013 07:02 AM, Neil Horman wrote:
> That does makes sense, but it then begs the question, whats the advantage of
> having multiple alu's at all?
There's lots of ALU operations that don't operate on the flags or other
entities that can be run in parallel.
> If they're just going to serialize on the
> updating of the condition register, there doesn't seem to be much advantage in
> having multiple alu's at all, especially if a common use case (parallelizing an
> operation on a large linear dataset) resulted in lower performance.
>
> /me wonders if rearranging the instructions into this order:
> adcq 0*8(src), res1
> adcq 1*8(src), res2
> adcq 2*8(src), res1
>
> would prevent pipeline stalls. That would be interesting data, and (I think)
> support your theory, Doug. I'll give that a try
Just to avoid spending too much time on various combinations, here are
the methods I've tried:
Original code
2 chains doing interleaved memory accesses
2 chains doing serial memory accesses (as above)
4 chains doing serial memory accesses
4 chains using 32bit values in 64bit registers so you can always use add
instead of adc and never need the carry flag
And I've done all of the above with simple prefetch and smart prefetch.
In all cases, the result is basically that the add method doesn't matter
much in the grand scheme of things, but the prefetch does, and smart
prefetch always beat simple prefetch.
My simple prefetch was to just go into the main while() loop for the
csum operation and always prefetch 5*64 into the future.
My smart prefetch looks like this:
static inline void prefetch_line(unsigned long *cur_line,
unsigned long *end_line,
size_t size)
{
size_t fetched = 0;
while (*cur_line <= *end_line && fetched < size) {
prefetch((void *)*cur_line);
*cur_line += cache_line_size();
fetched += cache_line_size();
}
}
static unsigned do_csum(const unsigned char *buff, unsigned len)
{
...
unsigned long cur_line = (unsigned long)buff &
~(cache_line_size() - 1);
unsigned long end_line = ((unsigned long)buff + len) &
~(cache_line_size() - 1);
...
/* Don't bother to prefetch the first line, we'll end up
stalling on
* it anyway, but go ahead and start the prefetch on the next 3 */
cur_line += cache_line_size();
prefetch_line(&cur_line, &end_line, cache_line_size() * 3);
odd = 1 & (unsigned long) buff;
if (unlikely(odd)) {
result = *buff << 8;
...
count >>= 1; /* nr of 32-bit words.. */
/* prefetch line #4 ahead of main loop */
prefetch_line(&cur_line, &end_line, cache_line_size());
if (count) {
...
while (count64) {
/* we are now prefetching line #5 ahead of
* where we are starting, and will stay 5
* ahead throughout the loop, at least
until
* we get to the end line and then
we'll stop
* prefetching */
prefetch_line(&cur_line, &end_line, 64);
ADDL_64;
buff += 64;
count64--;
}
ADDL_64_FINISH;
I was going to tinker today and tomorrow with this function once I get a
toolchain that will compile it (I reinstalled all my rhel6 hosts as f20
and I'm hoping that does the trick, if not I need to do more work):
#define ADCXQ_64 \
asm("xorq %[res1],%[res1]\n\t" \
"adcxq 0*8(%[src]),%[res1]\n\t" \
"adoxq 1*8(%[src]),%[res2]\n\t" \
"adcxq 2*8(%[src]),%[res1]\n\t" \
"adoxq 3*8(%[src]),%[res2]\n\t" \
"adcxq 4*8(%[src]),%[res1]\n\t" \
"adoxq 5*8(%[src]),%[res2]\n\t" \
"adcxq 6*8(%[src]),%[res1]\n\t" \
"adoxq 7*8(%[src]),%[res2]\n\t" \
"adcxq %[zero],%[res1]\n\t" \
"adoxq %[zero],%[res2]\n\t" \
: [res1] "=r" (result1), \
[res2] "=r" (result2) \
: [src] "r" (buff), [zero] "r" (zero), \
"[res1]" (result1), "[res2]" (result2))
and then I also wanted to try using both xmm and ymm registers and doing
64bit adds with 32bit numbers across multiple xmm/ymm registers as that
should parallel nicely. David, you mentioned you've tried this, how did
your experiment turn out and what was your method? I was planning on
doing regular full size loads into one xmm/ymm register, then using
pshufd/vshufd to move the data into two different registers, then
summing into a fourth register, and possible running two of those
pipelines in parallel.
^ permalink raw reply
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Doug Ledford @ 2013-10-30 13:22 UTC (permalink / raw)
To: David Laight, Neil Horman; +Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73C5@saturn3.aculab.com>
On 10/30/2013 08:18 AM, David Laight wrote:
>> /me wonders if rearranging the instructions into this order:
>> adcq 0*8(src), res1
>> adcq 1*8(src), res2
>> adcq 2*8(src), res1
>
> Those have to be sequenced.
>
> Using a 64bit lea to add 32bit quantities should avoid the
> dependencies on the flags register.
> However you'd need to get 3 of those active to beat a 64bit adc.
>
> David
>
>
>
Already done (well, something similar to what you mention above anyway),
doesn't help (although doesn't hurt either, even though it doubles the
number of adds needed to complete the same work). This is the code I
tested:
#define ADDL_64 \
asm("xorq %%r8,%%r8\n\t" \
"xorq %%r9,%%r9\n\t" \
"xorq %%r10,%%r10\n\t" \
"xorq %%r11,%%r11\n\t" \
"movl 0*4(%[src]),%%r8d\n\t" \
"movl 1*4(%[src]),%%r9d\n\t" \
"movl 2*4(%[src]),%%r10d\n\t" \
"movl 3*4(%[src]),%%r11d\n\t" \
"addq %%r8,%[res1]\n\t" \
"addq %%r9,%[res2]\n\t" \
"addq %%r10,%[res3]\n\t" \
"addq %%r11,%[res4]\n\t" \
"movl 4*4(%[src]),%%r8d\n\t" \
"movl 5*4(%[src]),%%r9d\n\t" \
"movl 6*4(%[src]),%%r10d\n\t" \
"movl 7*4(%[src]),%%r11d\n\t" \
"addq %%r8,%[res1]\n\t" \
"addq %%r9,%[res2]\n\t" \
"addq %%r10,%[res3]\n\t" \
"addq %%r11,%[res4]\n\t" \
"movl 8*4(%[src]),%%r8d\n\t" \
"movl 9*4(%[src]),%%r9d\n\t" \
"movl 10*4(%[src]),%%r10d\n\t" \
"movl 11*4(%[src]),%%r11d\n\t" \
"addq %%r8,%[res1]\n\t" \
"addq %%r9,%[res2]\n\t" \
"addq %%r10,%[res3]\n\t" \
"addq %%r11,%[res4]\n\t" \
"movl 12*4(%[src]),%%r8d\n\t" \
"movl 13*4(%[src]),%%r9d\n\t" \
"movl 14*4(%[src]),%%r10d\n\t" \
"movl 15*4(%[src]),%%r11d\n\t" \
"addq %%r8,%[res1]\n\t" \
"addq %%r9,%[res2]\n\t" \
"addq %%r10,%[res3]\n\t" \
"addq %%r11,%[res4]" \
: [res1] "=r" (result1), \
[res2] "=r" (result2), \
[res3] "=r" (result3), \
[res4] "=r" (result4) \
: [src] "r" (buff), \
"[res1]" (result1), "[res2]" (result2), \
"[res3]" (result3), "[res4]" (result4) \
: "r8", "r9", "r10", "r11" )
^ permalink raw reply
* Re: 3.12-rc7 regression - network panic from ipv6
From: Steffen Klassert @ 2013-10-30 13:09 UTC (permalink / raw)
To: David Miller; +Cc: mroos, hannes, linux-kernel, netdev
In-Reply-To: <20131029.174258.1677867676998240250.davem@davemloft.net>
On Tue, Oct 29, 2013 at 05:42:58PM -0400, David Miller wrote:
> From: Meelis Roos <mroos@linux.ee>
> Date: Tue, 29 Oct 2013 23:38:28 +0200 (EET)
>
> >> > Some bad news - in a system where 3.12-rc6 and earlier worked fine,
> >> > 3.12-rc7 panics or hangs repeatedly with network traffic (torrent being
> >> > good test). First there is BUG from ipv6 code, followed by panic.
> >>
> >> Could you do a bisect on this? There seems to be one commit for this
> >> particular function _decode_session6:
> >>
> >> commit bafd4bd4dcfa13145db7f951251eef3e10f8c278
> >> Author: Steffen Klassert <steffen.klassert@secunet.com>
> >> Date: Mon Sep 9 10:38:38 2013 +0200
> >>
> >> xfrm: Decode sessions with output interface.
> >>
> >> The output interface matching does not work on forward
> >> policy lookups, the output interface of the flowi is
> >> always 0. Fix this by setting the output interface when
> >> we decode the session.
> >>
> >> Maybe try to just revert this change locally and try again?
> >
> > Yes, just reverting this patch on top of rc7 gets rid of the problem for
> > me.
>
> Steffen please fix this or I'll have to revert.
I was a bit surprised that the skb has no dst_entry attached.
But in the reported case, ip6_frag_queue() removes the dst_entry
explicitly on all but the last received fragments. And unlike the
ipv4 case, it does not restore it before ip6_expire_frag_queue()
calls icmpv6_send().
I'm currently testing the patch below. Meelis, could you please
check if this patch fixes your problems?
Unfortunately I'm off without network access for the whole day
tomorrow. So in case the patch fixes the problems, I'd integrate it
into the final ipsec pull request for this release cycle on friday.
Subject: [PATCH] xfrm: Fix null pointer dereference when decoding sessions
On some codepaths the skb does not have a dst entry
when xfrm_decode_session() is called. So check for
a valid skb_dst() before dereferencing the device
interface index. We use 0 as the device index if
there is no valid skb_dst(), or at reverse decoding
we use skb_iif as device interface index.
Bug was introduced with git commit bafd4bd4dc
("xfrm: Decode sessions with output interface.").
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 6 +++++-
net/ipv6/xfrm6_policy.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4764ee4..e1a6393 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -104,10 +104,14 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
const struct iphdr *iph = ip_hdr(skb);
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
struct flowi4 *fl4 = &fl->u.ip4;
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl4, 0, sizeof(struct flowi4));
fl4->flowi4_mark = skb->mark;
- fl4->flowi4_oif = skb_dst(skb)->dev->ifindex;
+ fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
if (!ip_is_fragment(iph)) {
switch (iph->protocol) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index dd503a3..5f8e128 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -135,10 +135,14 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl6, 0, sizeof(struct flowi6));
fl6->flowi6_mark = skb->mark;
- fl6->flowi6_oif = skb_dst(skb)->dev->ifindex;
+ fl6->flowi6_oif = reverse ? skb->skb_iif : oif;
fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/4 net-next] net: phy: add Generic Netlink Ethernet switch configuration API
From: Felix Fietkau @ 2013-10-30 12:53 UTC (permalink / raw)
To: Jamal Hadi Salim, Florian Fainelli, Neil Horman
Cc: John Fastabend, netdev, David Miller, Sascha Hauer, John Crispin,
Jonas Gorski, Gary Thomas, Vlad Yasevich, Stephen Hemminger
In-Reply-To: <5270F161.80603@mojatatu.com>
On 2013-10-30 12:45, Jamal Hadi Salim wrote:
> On 10/29/13 05:34, Felix Fietkau wrote:
>> On 2013-10-28 23:53, Jamal Hadi Salim wrote:
>
>>
>> These are simple switches, why would they respond to ARP?
>> I suspect that you're attributing too much functionality to the switch
>> itself. Think of it as a device similar to the cheap unmanaged ones you
>> can buy in a shop and hook up to your machine via Ethernet.
>> Add to that some very limited VLAN grouping functionality, and you're
>> pretty close to the limits of what these switches can do.
>> They don't do ARP, IP or other things. They learn about MAC addresses
>> from incoming packets to build their forwarding path.
>> The CPU port in this case is whatever port on the switch that you plug
>> the cable of your machine into :)
>
> Ok, got it - the only use for cpu for these things is to retrieve things
> like stats, link state, etc; can you even read the fdb?
Where supported, all you can typically read is a list of which MAC
address was discovered behind which port - if you're lucky. You usually
won't find VLAN information attached to that.
Often it simply isn't supported at all.
>> The FDB related abstraction that you're describing will not work with
>> the hardware that I'm talking about. Let's leave that one out of this
>> discussion.
>
> sigh - ok. But you gotta help me understand why.
The hardware implementation of MAC address handling isn't even
consistent across chips from different vendors. Often you don't even get
things like the VLAN ID. Sometimes there's a global forwarding table,
sometimes you can have multiple tables and assign them to VLANs.
>>> Can we call that "L3" instead of software bridge?
>> L3? Why?
>
> We have two L2 domains. You want to connect them - you need a higher
> layer; Layer 3 seems to be the simple one (i.e typically people would
> use ip to link two layer 2 broadcast domains).
If you connect two L2 domains through a bridge, I still consider that L2
- it's still on the same layer, just goes through more hops.
>> I think that's way more confusing to users than presenting a consistent
>> model that properly reflects what you can do with the hardware.
> I think discovery from a control view is always a win.
Yes, and swconfig handles the discovery part fairly well.
>> I'm not going to try to enumerate all the case; I have other projects
>> that I need to work on. :)
>
> I understand. I am busy as well, just saying if we need to reach an
> agreement to either agree or disagree we need to capture the esoterics
> of the different cases; as you can see i tried to enumerate some in
> my previous email. In my case this would be useful to see, using current
> mechanisms, that it can or cant be done or can be done with mods etc.
At this point, I'm not sure if we will be able to reach an agreement. I
think I've shown over and over again that what you're proposing comes
with huge costs in terms of complexity and bloat, as demonstrated by the
fact that it adds so many corner cases that would have to be dealt with,
including many for which we haven't even the slightest idea of a good
solution.
Now, to make this a viable option, the benefits would have to be big and
significant enough to offset these costs.
The only real benefit you've pointed out so far is to be able to reuse
existing tools/APIs (but only with modifications, not as-is). I think
that's fairly small, when put in perspective with the hard problems that
this approach creates, both for users (hidden traps and surprises) and
for developers (implementation difficulties and incompatible abstractions).
>> Only a *tiny* part of the software bridge configuration model can be
>> emulated, the rest does not fit and has to be handled through extensions
>> or different APIs anyway. That's why I am convinced that it's a really
>> bad model to try to make these switches fit into it.
>>
>> You gain a tiny advantage with writing scripts, but at the same time,
>> the code gets more complex, the configuration interface gets more
>> confusing, there are more nasty corner cases to take care of.
>> Why do you insist on making so many things worse just for one tiny
>> advantage? Where's the pragmatic cost/benefit tradeoff?
>>
>
> There is nothing wrong with making extensions if they make sense.
Yes, but if the basic abstraction doesn't make sense for the use case,
and it leads to too many corner cases, there's everything wrong with
trying to work around that through extensions.
> My problem so far in this discussion is i havent figured which will be bad
> extensions you bring up. My approach is to list things and
> then point out which one will require some witchcraft on top of
> current interfaces. I am afraid I am still missing that part. Maybe
> I have to go back and study your patch some more.
Sure, go ahead.
>> On pretty much all devices that we work with, one of the ports
>> connects to a NIC in the CPU. It's just that the switch cannot be
>> assumed to have special treatment for that CPU port. As far as it is
>> concerned, it is just another port like the others.
>
> Aha. I think i see a small terminology cross-talk. You refer to things
> as NICs when i use the term netdev. So now i understand better what you
> mean by rx handler (I intepreted earlier to mean something at the tap
> level).
I only started using the term NIC to emphasize that it's not just a
netdev of the switch - it's a real Ethernet MAC (usually in the SoC),
with a separate driver that knows nothing about the switch.
> Ok, so Felix, for the case where we have switches with cpu ports
> that can tag incoming packets with ingress port ids - can we say the
> NIC rx handler is reasonable to be used as a demux point for the
> software version of the ports? I am not talking about the corner
> cases.
Yes, but when looking at the big picture, the switch being able to tag
incoming packets with the ingress port is a corner case!
Most switches that we work with aren't actually able to do that!
I want to have a decent baseline implementation that does not assume
this port tagging capability.
>>> - ive never seen table id, but i think this is another one; in which
>>> case the number of table ids becomes something one needs to discover..
>> Yes, and this is something that doesn't even map directly to something
>> in the software bridge world.
> It does - There is a single table per bridge on the software bridge
> world. You need multiple bridges, one per id.
Depends on which software bridge.
If I have two normal netdevs, eth0 and eth1, I can create eth0.4 and
bridge it to eth1.5. That's just one bridge.
I can't easily emulate that with fake per-port netdevs and a typical
switch supported by swconfig.
With just swconfig (no fake netdevs) switches that support these table
ids, I would need to have two VLANs in the switch (both connected to the
CPU port, each one getting a separate table id), and then one software
bridge between eth0.4 and eth0.5 (assuming eth0 connects to the switch).
- Felix
^ permalink raw reply
* RE: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: David Laight @ 2013-10-30 12:18 UTC (permalink / raw)
To: Neil Horman, Doug Ledford; +Cc: Ingo Molnar, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <20131030110214.GA10220@localhost.localdomain>
> /me wonders if rearranging the instructions into this order:
> adcq 0*8(src), res1
> adcq 1*8(src), res2
> adcq 2*8(src), res1
Those have to be sequenced.
Using a 64bit lea to add 32bit quantities should avoid the
dependencies on the flags register.
However you'd need to get 3 of those active to beat a 64bit adc.
David
^ permalink raw reply
* Re: [PATCH 1/4 net-next] net: phy: add Generic Netlink Ethernet switch configuration API
From: Felix Fietkau @ 2013-10-30 11:58 UTC (permalink / raw)
To: Jamal Hadi Salim, mbizon
Cc: Florian Fainelli, Neil Horman, John Fastabend, netdev,
David Miller, Sascha Hauer, John Crispin, Jonas Gorski,
Gary Thomas, Vlad Yasevich, Stephen Hemminger
In-Reply-To: <5270F282.6000708@mojatatu.com>
On 2013-10-30 12:50, Jamal Hadi Salim wrote:
> On 10/29/13 19:12, Maxime Bizon wrote:
>
>>
>> From a user POV, when you see a netdevice, you expect to be able to
>> receive or send packets from/to it. The ability to read stats/link is
>> only a secondary feature.
>>
>
> The important part is all the APIs stay consistent. I can use
> same netlink calls. ifconfig works.
> iproute2 works. People have written books on this stuff - we dont
> have MCSE(Must Call Software Engineer) certification, but this is
> as close as it gets. i.e the knowledge has been commoditized, even
> my kid knows how to use these tools.
>
> If i can get stats by doing ifconfig - that should provide illusion that
> the netdevice is sending/receiving packets.
Pretty much all of the above have serious limitations when you're not
actually able to run the data path through the per-port netdevs.
You can't assign IP addresses to them. The network stack will probably
even attempt to assign IPv6 link-local addresses to these things,
causing even more confusion.
You can't add them to normal software bridges like other devices.
You can't use bonding. I could probably go on for a while.
There's a huge list of things that you simply cannot do with these
interfaces, and without knowing the details of the implementation, users
will be left clueless as to why that is.
I'd say that's a very serious violation of the principle of least surprise.
And knowing what the typical OpenWrt users do with their devices, I can
already forsee the bogus bug reports trickling in, if this is to be
implemented.
- Felix
^ permalink raw reply
* Re: [PATCH 14/16] wl1251: add nvs file name to module firmware list
From: Pavel Machek @ 2013-10-30 11:55 UTC (permalink / raw)
To: Pali Rohár
Cc: Luciano Coelho, John W. Linville, Johannes Berg, David S. Miller,
linux-wireless, netdev, linux-kernel, freemangordon,
aaro.koskinen, sre, joni.lapilainen
In-Reply-To: <1382819655-30430-15-git-send-email-pali.rohar@gmail.com>
On Sat 2013-10-26 22:34:13, Pali Rohár wrote:
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ 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