* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
From: Alexander Duyck @ 2017-01-23 16:28 UTC (permalink / raw)
To: David Laight, Tushar Dave
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0268E25@AcuExch.aculab.com>
On Mon, Jan 23, 2017 at 1:44 AM, David Laight <David.Laight@aculab.com> wrote:
> Alexander Duyck
>> Sent: 19 January 2017 15:55
> ...
>> >> The Relaxed Ordering attribute doesn't get applied across the board.
>> >> It ends up being limited to a subset of the transactions if I recall
>> >> correctly. In this case it is the Tx descriptor write back, and the
>> >> Rx data write back. We don't apply the RO bit to any other
>> >> transactions.
>> >>
>> >> In the case of Tx descriptor there is no harm in allowing it to be
>> >> reordered because we only really read the DD bit so we don't care
>> >> about the ordering of the write back. In the case of the Rx data the
>> >> Rx descriptor essentially acts as a flush since it is sent without the
>> >> RO bit set. So all the writes before it must be completed before the
>> >> Rx descriptor write back.
>> >
>> > In which case why not set it unconditionally for all architectures?
>> >
>> > I'm surprised (I often am) that allowing those re-orderings makes
>> > any significant difference.
>> > Unfortunately you need a PCIe analyser to see what is really happening
>> > and they don't come cheap.
>> >
>> > What I do vaguely remember is that some hosts don't always implement
>> > the 'normal' re-ordering of reads and read completions.
>> > Re-ordering of reads allows descriptor reads to overtake transmit
>> > traffic which is likely to make a difference.
>>
>> I think part of the issue, at least in the case of SPARC, is that the
>> handling of the memory writes in the PCIe root complex is impacted by
>> the RO attribute. On the bus itself it doesn't matter much, but at
>> the root complex it can become expensive to have to wait on a partial
>> write to complete while there are other writes pending. This is why
>> the IOMMU for SPARC now has a WEAK_ORDERING attribute you can add so
>> that it can write the data in whatever order it wants in relation to
>> other writes in that region.
>
> I hope the IOMMU only ever reorders writes that have the RO bit set.
I'm assuming it only applies to DMA regions mapped with
DMA_ATTR_WEAK_ORDERING. Since drivers have to specify that attribute
it likely is only going to apply to DMA regions that could have the RO
bit set.
> Has anyone tried cache invalidates on the rx buffers?
> Might make the writes less expensive.
> Or is the issue with NUMA rather than cache.
I don't know. This is all very SPARC specific and I haven't done any
of the work on it. You might try checking with those responsible for
introducing DMA_ATTR_WEAK_ORDERING for the SPARC architecture.
- Alex
^ permalink raw reply
* Re: [PATCH] phy: marvell: remove conflicting initializer
From: Andrew Lunn @ 2017-01-23 16:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Fainelli, David S. Miller, Charles-Antoine Couret,
Clemens Gruber, netdev, linux-kernel
In-Reply-To: <7242508.n9OCrlbXLu@wuerfel>
> Great question! I have sent out patches for a couple of actual bugs that
> came from this warning in the past few months.
>
> I still have a couple of patches that I have not sent out so far,
> I think they are all false-positives, and I have not found a good
> workaround for most of them other than disabling the warning locally.
>
> Once we get the patches below (or some other workaround) into the
> kernel, we should definitely enable the warning by default.
So it looks like some are from initialising a range of values with
a default value:
[0 ... SH_ETH_MAX_REGISTER_OFFSET - 1] = SH_ETH_OFFSET_INVALID
and then some entries specific values. Maybe the compiler can help out
here? A different warning in this case? And have that at W=1? Could
you ask the gcc folks about this?
Others causes seem like
226 #define DRM_FB_HELPER_DEFAULT_OPS \
227 .fb_check_var = drm_fb_helper_check_var, \
228 .fb_set_par = drm_fb_helper_set_par, \
229 .fb_setcmap = drm_fb_helper_setcmap, \
230 .fb_blank = drm_fb_helper_blank, \
231 .fb_pan_display = drm_fb_helper_pan_display
static struct fb_ops omap_fb_ops = {
.owner = THIS_MODULE,
DRM_FB_HELPER_DEFAULT_OPS,
...
.fb_pan_display = omap_fbdev_pan_display,
There are at least three drivers which need to replace the
default fb_pan_display. So maybe a different macro?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/2] vxlan: don't flush static fdb entries on admin down
From: Roopa Prabhu @ 2017-01-23 16:31 UTC (permalink / raw)
To: Jiri Benc; +Cc: davem, netdev, ramanb, stephen, pshelar
In-Reply-To: <20170123095555.0778abb6@griffin>
On 1/23/17, 12:55 AM, Jiri Benc wrote:
> On Fri, 20 Jan 2017 23:43:18 -0800, Roopa Prabhu wrote:
>> This patch skips flushing static fdb entries in
>> ndo_stop, but flushes all fdb entries during vxlan
>> device delete. This is consistent with the bridge
>> driver fdb
> This makes sense but isn't this a uAPI change? Do you know whether
> there are users relying on the current behavior?
>
yeah, i have debated about this also. I am sitting on a stash of such patches
which are borderline bug fixes but been out there long enough that people might care.
I leaned towards making it the default because,
- I am assuming not many people use static entries
- and people will not be very unhappy with this change....because it does not delete something
they had...just retains them.
- Per device flag seems like an overkill here
Also, i think some such changes were done to the bridge driver too after it was out.
^ permalink raw reply
* Re: [PATCH v4 1/3] bpf: add a longest prefix match trie map implementation
From: Daniel Borkmann @ 2017-01-23 16:39 UTC (permalink / raw)
To: Daniel Mack, ast; +Cc: dh.herrmann, netdev, davem
In-Reply-To: <20170121162613.4159-2-daniel@zonque.org>
On 01/21/2017 05:26 PM, Daniel Mack wrote:
[...]
> +/* Called from syscall or from eBPF program */
> +static int trie_update_elem(struct bpf_map *map,
> + void *_key, void *value, u64 flags)
> +{
> + struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
> + struct lpm_trie_node *node, *im_node, *new_node = NULL;
im_node is uninitialized here ...
> + struct lpm_trie_node __rcu **slot;
> + struct bpf_lpm_trie_key *key = _key;
> + unsigned long irq_flags;
> + unsigned int next_bit;
> + size_t matchlen = 0;
> + int ret = 0;
> +
> + if (unlikely(flags > BPF_EXIST))
> + return -EINVAL;
> +
> + if (key->prefixlen > trie->max_prefixlen)
> + return -EINVAL;
> +
> + raw_spin_lock_irqsave(&trie->lock, irq_flags);
> +
> + /* Allocate and fill a new node */
> +
> + if (trie->n_entries == trie->map.max_entries) {
> + ret = -ENOSPC;
> + goto out;
... and here we go to out path with ret as non-zero ...
> + }
> +
> + new_node = lpm_trie_node_alloc(trie, value);
> + if (!new_node) {
> + ret = -ENOMEM;
> + goto out;
> + }
[...]
> +
> +out:
> + if (ret) {
> + if (new_node)
> + trie->n_entries--;
> +
> + kfree(new_node);
> + kfree(im_node);
... which does kfree() in im_node here.
> + }
> +
> + raw_spin_unlock_irqrestore(&trie->lock, irq_flags);
> +
> + return ret;
> +}
^ permalink raw reply
* Re: [PATCH v4 1/3] bpf: add a longest prefix match trie map implementation
From: Daniel Mack @ 2017-01-23 16:42 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: dh.herrmann, netdev, davem
In-Reply-To: <588631CE.9080402@iogearbox.net>
On 01/23/2017 05:39 PM, Daniel Borkmann wrote:
> On 01/21/2017 05:26 PM, Daniel Mack wrote:
> [...]
>> +/* Called from syscall or from eBPF program */
>> +static int trie_update_elem(struct bpf_map *map,
>> + void *_key, void *value, u64 flags)
>> +{
>> + struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
>> + struct lpm_trie_node *node, *im_node, *new_node = NULL;
>
> im_node is uninitialized here ...
>
>> + struct lpm_trie_node __rcu **slot;
>> + struct bpf_lpm_trie_key *key = _key;
>> + unsigned long irq_flags;
>> + unsigned int next_bit;
>> + size_t matchlen = 0;
>> + int ret = 0;
>> +
>> + if (unlikely(flags > BPF_EXIST))
>> + return -EINVAL;
>> +
>> + if (key->prefixlen > trie->max_prefixlen)
>> + return -EINVAL;
>> +
>> + raw_spin_lock_irqsave(&trie->lock, irq_flags);
>> +
>> + /* Allocate and fill a new node */
>> +
>> + if (trie->n_entries == trie->map.max_entries) {
>> + ret = -ENOSPC;
>> + goto out;
>
> ... and here we go to out path with ret as non-zero ...
>
>> + }
>> +
>> + new_node = lpm_trie_node_alloc(trie, value);
>> + if (!new_node) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
> [...]
>> +
>> +out:
>> + if (ret) {
>> + if (new_node)
>> + trie->n_entries--;
>> +
>> + kfree(new_node);
>> + kfree(im_node);
>
> ... which does kfree() in im_node here.
Oops. Nice catch! gcc was too stupid to recognize that :)
Thanks, I'll repost a v5 with Alexei's Acked-by later today.
Daniel
^ permalink raw reply
* RE: [PATCH 0/4] Make xfrm usable by 32-bit programs
From: David Laight @ 2017-01-23 16:45 UTC (permalink / raw)
To: 'Kevin Cernekee', steffen.klassert@secunet.com,
herbert@gondor.apana.org.au, davem@davemloft.net,
paul@paul-moore.com, sds@tycho.nsa.gov, eparis@parisplace.org
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
selinux@tycho.nsa.gov, fw@strlen.de, fan.du@windriver.com,
dianders@chromium.org, dtor@chromium.org
In-Reply-To: <20170121000507.34381-1-cernekee@chromium.org>
From: Kevin Cernekee
> Sent: 21 January 2017 00:05
> Several of the xfrm netlink and setsockopt() interfaces are not usable
> from a 32-bit binary running on a 64-bit kernel due to struct padding
> differences. This has been the case for many, many years[0]. This
> patch series deprecates the broken netlink messages and replaces them
> with packed structs that are compatible between 64-bit and 32-bit
> programs.
Do you mean '__packed' or just structures with no holes?
The latter is really the best.
You can add a compile time assert on the structure sizes to ensure
that the user API is never accidentally broken.
I can't imagine any reason to change the 64bit structures except
(maybe) to make any padding explicit.
> It retains support for legacy user programs (i.e. anything
> that is currently working today), and allows legacy support to be
> compiled out via CONFIG_XFRM_USER_LEGACY if it becomes unnecessary in
> the future.
...
Isn't that effectively the same as doing a compat layer?
Otherwise you can't build a 32bit app that will work with an 'old'
32bit kernel and a new 64bit one.
Provided you've got the length of the user's buffer the compat code
ought to be trivial (if tedious).
David
^ permalink raw reply
* Re: [PATCH net-next] net: ipv6: ignore null_entry on route dumps
From: David Ahern @ 2017-01-23 16:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, xiaolong.ye
In-Reply-To: <20170122.235829.1911961579892347071.davem@davemloft.net>
On 1/22/17 9:58 PM, David Miller wrote:
>
> David, please slow down.
>
> How is the NULL entry getting selected to be dumped and passed
> down here in the first place?
The null_entry might be a 'marker' but is integral to the ipv6 fib. For starters:
static int __net_init fib6_net_init(struct net *net)
{
...
net->ipv6.fib6_main_tbl = kzalloc(sizeof(*net->ipv6.fib6_main_tbl),
GFP_KERNEL);
...
net->ipv6.fib6_main_tbl->tb6_id = RT6_TABLE_MAIN;
net->ipv6.fib6_main_tbl->tb6_root.leaf = net->ipv6.ip6_null_entry;
...
Same goes for every other table allocated:
static struct fib6_table *fib6_alloc_table(struct net *net, u32 id)
{
struct fib6_table *table;
table = kzalloc(sizeof(*table), GFP_ATOMIC);
if (table) {
table->tb6_id = id;
table->tb6_root.leaf = net->ipv6.ip6_null_entry;
..
}
And if you look at the fib6_add code:
sfn->leaf = info->nl_net->ipv6.ip6_null_entry;
atomic_inc(&info->nl_net->ipv6.ip6_null_entry->rt6i_ref);
sfn->fn_flags = RTN_ROOT;
sfn->fn_sernum = sernum;
/* Now add the first leaf node to new subtree */
sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
allow_create, replace_required, sernum);
Meaning that a walk of the table is going to dump the ip6_null_entry route, potentially multiple times.
As far as the dump request the chain rtnetlink message invokes inet6_dump_fib:
inet6_dump_fib
- fib6_dump_table
+ fib6_walk
* fib6_walk_continue
- fib6_dump_node
+ rt6_dump_route
Seems to me the most appropriate place to check that rt is the null_entry route is rt6_dump_route but perhaps fib6_dump_node could as well. This patch buts the check in rt6_dump_route.
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Jason Baron @ 2017-01-23 16:56 UTC (permalink / raw)
To: Oleg Nesterov, David S. Miller
Cc: Herbert Xu, Yauheni Kaliuta, netdev, Eric Dumazet
In-Reply-To: <20170123143025.GA31676@redhat.com>
On 01/23/2017 09:30 AM, Oleg Nesterov wrote:
> Hello,
>
> smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
> non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
> (non-atomic too) won't be reordered.
>
Indeed. Here's a bit of discussion on it:
http://marc.info/?l=linux-netdev&m=146662325920596&w=2
> It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> and the patch looks correct in that we need the barriers in tcp_check_space()
> and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
>
Yes, I think it should be upgraded to an smp_mb() there. If you agree
with this analysis, I will send a patch to upgrade it. Note, I did not
actually run into this race in practice.
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH 0/4] Make xfrm usable by 32-bit programs
From: David Miller @ 2017-01-23 16:57 UTC (permalink / raw)
To: David.Laight
Cc: cernekee, steffen.klassert, herbert, paul, sds, eparis,
linux-kernel, netdev, selinux, fw, fan.du, dianders, dtor
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026BC58@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Mon, 23 Jan 2017 16:45:39 +0000
> Provided you've got the length of the user's buffer the compat code
> ought to be trivial (if tedious).
Wireless guys had to deal with a similar problem with nl80211.
You don't know who is going to get the message when you build it,
because you can't possibly know what applications are listening
on the netlink sockets for notifications and even if you did
that set changes dynamically and could be different by the time
the notification is delivered.
The long and short of it is that you must always generate both the
native 64-bit as well as the 32-bit compat version of every message,
then at delivery time you enqueue the appropriate one to each
receiving socket.
This is how nl80211 solved the problem, and it's what XFRM should do
as well.
^ permalink raw reply
* [RFC PATCH net-next 2/5] net: split skb_checksum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
skb_checksum_help is designed to compute the Internet Checksum only. To
avoid duplicating code when other checksumming algorithms (e.g. crc32c)
are used, separate common part from RFC1624-specific part.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/core/dev.c | 51 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index ad5959e..6742160 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2532,13 +2532,36 @@ static void skb_warn_bad_offload(const struct sk_buff *skb)
skb_shinfo(skb)->gso_type, skb->ip_summed);
}
-/*
- * Invalidate hardware checksum when packet is to be mangled, and
+/* compute 16-bit RFC1624 checksum and store it at skb->data + offset */
+static int skb_rfc1624_csum(struct sk_buff *skb, int offset)
+{
+ __wsum csum;
+ int ret = 0;
+
+ csum = skb_checksum(skb, offset, skb->len - offset, 0);
+
+ offset += skb->csum_offset;
+ BUG_ON(offset + sizeof(__sum16) > skb_headlen(skb));
+
+ if (skb_cloned(skb) &&
+ !skb_clone_writable(skb, offset + sizeof(__sum16))) {
+ ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (ret)
+ goto out;
+ }
+ *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
+out:
+ return ret;
+}
+
+/* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
+ * @skb - buffer that needs checksum
+ * @csum_algo(skb, offset) - function used to compute the checksum
*/
-int skb_checksum_help(struct sk_buff *skb)
+static int __skb_checksum_help(struct sk_buff *skb,
+ int (*csum_algo)(struct sk_buff *, int))
{
- __wsum csum;
int ret = 0, offset;
if (skb->ip_summed == CHECKSUM_COMPLETE)
@@ -2560,24 +2583,20 @@ int skb_checksum_help(struct sk_buff *skb)
offset = skb_checksum_start_offset(skb);
BUG_ON(offset >= skb_headlen(skb));
- csum = skb_checksum(skb, offset, skb->len - offset, 0);
-
- offset += skb->csum_offset;
- BUG_ON(offset + sizeof(__sum16) > skb_headlen(skb));
-
- if (skb_cloned(skb) &&
- !skb_clone_writable(skb, offset + sizeof(__sum16))) {
- ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
- if (ret)
- goto out;
- }
- *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
+ ret = csum_algo(skb, offset);
+ if (ret)
+ goto out;
out_set_summed:
skb->ip_summed = CHECKSUM_NONE;
out:
return ret;
}
+
+int skb_checksum_help(struct sk_buff *skb)
+{
+ return __skb_checksum_help(skb, skb_rfc1624_csum);
+}
EXPORT_SYMBOL(skb_checksum_help);
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 3/5] net: introduce skb_sctp_csum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
skb_sctp_csum_help is like skb_checksum_help, but it is designed for
checksumming SCTP packets using crc32c (see RFC3309), provided that
sctp.ko has been loaded before. In case sctp.ko is not loaded, invoking
skb_sctp_csum_help() on a skb results in the following printout:
sk_buff: attempt to compute crc32c without sctp.ko
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 3 ++-
net/core/dev.c | 29 +++++++++++++++++++++++++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3868c32..9d72824 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3902,6 +3902,7 @@ void netdev_rss_key_fill(void *buffer, size_t len);
int dev_get_nest_level(struct net_device *dev);
int skb_checksum_help(struct sk_buff *skb);
+int skb_sctp_csum_help(struct sk_buff *skb);
struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
netdev_features_t features, bool tx_path);
struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 44fc804..91b4e22 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -192,7 +192,8 @@
* accordingly. Note the there is no indication in the skbuff that the
* CHECKSUM_PARTIAL refers to an SCTP checksum, a driver that supports
* both IP checksum offload and SCTP CRC offload must verify which offload
- * is configured for a packet presumably by inspecting packet headers.
+ * is configured for a packet presumably by inspecting packet headers; in
+ * case, skb_sctp_csum_help is provided to compute CRC on SCTP packets.
*
* NETIF_F_FCOE_CRC - This feature indicates that a device is capable of
* offloading the FCOE CRC in a packet. To perform this offload the stack
diff --git a/net/core/dev.c b/net/core/dev.c
index 6742160..45cee84 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2554,6 +2554,29 @@ static int skb_rfc1624_csum(struct sk_buff *skb, int offset)
return ret;
}
+/* compute 32-bit RFC3309 checksum and store it at skb->data + offset */
+static int skb_rfc3309_csum(struct sk_buff *skb, int offset)
+{
+ __le32 crc32c_csum;
+ int ret = 0;
+
+ crc32c_csum = cpu_to_le32(~__skb_checksum(skb, offset,
+ skb->len - offset, ~(__u32)0,
+ sctp_csum_stub));
+ offset += skb->csum_offset;
+ BUG_ON((offset + sizeof(__le32)) > skb_headlen(skb));
+
+ if (skb_cloned(skb) &&
+ !skb_clone_writable(skb, offset + sizeof(__le32))) {
+ ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (ret)
+ goto out;
+ }
+ *(__le32 *)(skb->data + offset) = crc32c_csum;
+out:
+ return ret;
+}
+
/* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
* @skb - buffer that needs checksum
@@ -2599,6 +2622,12 @@ int skb_checksum_help(struct sk_buff *skb)
}
EXPORT_SYMBOL(skb_checksum_help);
+int skb_sctp_csum_help(struct sk_buff *skb)
+{
+ return __skb_checksum_help(skb, skb_rfc3309_csum);
+}
+EXPORT_SYMBOL(skb_sctp_csum_help);
+
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
__be16 type = skb->protocol;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 4/5] net: more accurate checksumming in validate_xmit_skb
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
introduce skb_csum_hwoffload_help and use it as a replacement for
skb_checksum_help in validate_xmit_skb, to compute checksum using crc32c or
2-complement Internet Checksum (or leave the packet unchanged and let the
NIC do the checksum), depending on netdev checksum offloading capabilities
and on presence of IPPROTO_SCTP as protocol number in IPv4/IPv6 header.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/core/dev.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 45cee84..f8cb3ba 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -140,6 +140,7 @@
#include <linux/hrtimer.h>
#include <linux/netfilter_ingress.h>
#include <linux/crash_dump.h>
+#include <linux/sctp.h>
#include "net-sysfs.h"
@@ -2960,6 +2961,54 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
return skb;
}
+static int skb_csum_hwoffload_help(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ bool encap = skb->encapsulation;
+ unsigned int offset = 0;
+ __be16 protocol;
+
+ if (likely((features & (NETIF_F_SCTP_CRC | NETIF_F_CSUM_MASK)) ==
+ (NETIF_F_SCTP_CRC | NETIF_F_CSUM_MASK)))
+ return 0;
+
+ if (skb->csum_offset != offsetof(struct sctphdr, checksum))
+ goto inet_csum;
+
+ if (encap) {
+ protocol = skb->inner_protocol;
+ if (skb->inner_protocol_type == ENCAP_TYPE_IPPROTO)
+ switch (protocol) {
+ case IPPROTO_IPV6:
+ protocol = ntohs(ETH_P_IPV6);
+ break;
+ case IPPROTO_IP:
+ protocol = ntohs(ETH_P_IP);
+ break;
+ default:
+ goto inet_csum;
+ }
+ } else {
+ protocol = vlan_get_protocol(skb);
+ }
+ switch (protocol) {
+ case ntohs(ETH_P_IP):
+ if ((encap ? inner_ip_hdr(skb) : ip_hdr(skb))->protocol ==
+ IPPROTO_SCTP)
+ goto sctp_csum;
+ break;
+ case ntohs(ETH_P_IPV6):
+ if (ipv6_find_hdr(skb, &offset, IPPROTO_SCTP, NULL, NULL) ==
+ IPPROTO_SCTP)
+ goto sctp_csum;
+ break;
+ }
+inet_csum:
+ return !(features & NETIF_F_CSUM_MASK) ? skb_checksum_help(skb) : 0;
+sctp_csum:
+ return !(features & NETIF_F_SCTP_CRC) ? skb_sctp_csum_help(skb) : 0;
+}
+
static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
{
netdev_features_t features;
@@ -2995,8 +3044,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
else
skb_set_transport_header(skb,
skb_checksum_start_offset(skb));
- if (!(features & NETIF_F_CSUM_MASK) &&
- skb_checksum_help(skb))
+ if (skb_csum_hwoffload_help(skb, features))
goto out_kfree_skb;
}
}
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 0/5] net: improve support for SCTP checksums
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
When NETIF_F_CSUM_MASK bits are all 0 on netdev features, validate_xmit_skb
uses skb_checksum_help to compute 16-bit, 2-complement checksum on non-GSO
skbs having ip_summed equal to CHECKSUM_PARTIAL.
This results in a systematic corruption of SCTP packets, since they need
to be checksummed with crc32c. Moreover, this is done regardless the value
of NETIF_F_SCTP_CRC, so any chance to offload crc32c computation on the
NIC is lost. Finally, even when at least one bit in NETIF_F_CSUM_MASK is
set on netdev features, validate_xmit_skb skips checksum computation - but
then most NIC drivers can only call skb_checksum_help if their HW can't
offload the checksum computation. Depending on the driver code, this
results in wrong handling of SCTP, leading to:
- packet being dropped
- packet being transmitted with identically-zero checksum
- packet being transmitted with 2-complement checksum instead of crc32c
This series tries to address the above issue, by providing:
- the possibility to compute crc32c on skbs in Linux net core [patch 1]
- skb_sctp_csum_help, a function sharing common code with the original
skb_checksum_help, that performs SW checksumming for skbs using crc32c
[patch 2 and patch 3]
- skb_csum_hwoffload_help, called by validate xmit skb to perform SW
checksumming using the correct algorithm based on the value of IP
protocol number and netdev features bitmask [patch 4]
- an update to Linux documentation [patch 5]
Davide Caratti (5):
skbuff: add stub to help computing crc32c on SCTP packets
net: split skb_checksum_help
net: introduce skb_sctp_csum_help
net: more accurate checksumming in validate_xmit_skb
Documentation: add description of skb_sctp_csum_help
Documentation/networking/checksum-offloads.txt | 9 +-
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 5 +-
net/core/dev.c | 132 +++++++++++++++++++++----
net/core/skbuff.c | 20 ++++
net/sctp/offload.c | 7 ++
6 files changed, 151 insertions(+), 23 deletions(-)
--
2.7.4
^ permalink raw reply
* [RFC PATCH net-next 5/5] Documentation: add description of skb_sctp_csum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
Add description of skb_sctp_csum_help in networking/checksum-offload.txt;
while at it, remove reference to skb_csum_off_chk* functions, since they
are not present anymore in Linux since commit cf53b1da73bd ('Revert "net:
Add driver helper functions to determine checksum"').
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
Documentation/networking/checksum-offloads.txt | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
index 56e3686..cb7a7e5 100644
--- a/Documentation/networking/checksum-offloads.txt
+++ b/Documentation/networking/checksum-offloads.txt
@@ -49,9 +49,9 @@ A driver declares its offload capabilities in netdev->hw_features; see
and csum_offset given in the SKB; if it tries to deduce these itself in
hardware (as some NICs do) the driver should check that the values in the
SKB match those which the hardware will deduce, and if not, fall back to
- checksumming in software instead (with skb_checksum_help or one of the
- skb_csum_off_chk* functions as mentioned in include/linux/skbuff.h). This
- is a pain, but that's what you get when hardware tries to be clever.
+ checksumming in software instead (with skb_checksum_help or
+ skb_sctp_csum_help functions as mentioned in include/linux/skbuff.h).
+ This is a pain, but that's what you get when hardware tries to be clever.
The stack should, for the most part, assume that checksum offload is
supported by the underlying device. The only place that should check is
@@ -60,7 +60,8 @@ The stack should, for the most part, assume that checksum offload is
may include other offloads besides TX Checksum Offload) and, if they are
not supported or enabled on the device (determined by netdev->features),
performs the corresponding offload in software. In the case of TX
- Checksum Offload, that means calling skb_checksum_help(skb).
+ Checksum Offload, that means calling skb_sctp_csum_help(skb) for SCTP
+ packets, and skb_checksum_help(skb) for other packets.
LCO: Local Checksum Offload
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 1/5] skbuff: add stub to help computing crc32c on SCTP packets
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
it can't be used in net core. Like it has been done previously with other
symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
to allow computation of SCTP checksum in net core after sctp.ko (and thus
libcrc32c) has been loaded.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/skbuff.h | 2 ++
net/core/skbuff.c | 20 ++++++++++++++++++++
net/sctp/offload.c | 7 +++++++
3 files changed, 29 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6f63b7e..44fc804 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3119,6 +3119,8 @@ struct skb_checksum_ops {
__wsum (*combine)(__wsum csum, __wsum csum2, int offset, int len);
};
+extern const struct skb_checksum_ops *sctp_csum_stub __read_mostly;
+
__wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum csum, const struct skb_checksum_ops *ops);
__wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f8dbe4a..60e9963 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2235,6 +2235,26 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
}
EXPORT_SYMBOL(skb_copy_and_csum_bits);
+static __wsum warn_sctp_csum_update(const void *buff, int len, __wsum sum)
+{
+ net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
+ return 0;
+}
+
+static __wsum warn_sctp_csum_combine(__wsum csum, __wsum csum2,
+ int offset, int len)
+{
+ net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
+ return 0;
+}
+
+const struct skb_checksum_ops *sctp_csum_stub __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = warn_sctp_csum_update,
+ .combine = warn_sctp_csum_combine,
+};
+EXPORT_SYMBOL(sctp_csum_stub);
+
/**
* skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
* @from: source buffer
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 7e869d0..1c9c548 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -98,6 +98,12 @@ static const struct net_offload sctp6_offload = {
},
};
+static const struct skb_checksum_ops *sctp_csum_ops __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = sctp_csum_update,
+ .combine = sctp_csum_combine,
+};
+
int __init sctp_offload_init(void)
{
int ret;
@@ -110,6 +116,7 @@ int __init sctp_offload_init(void)
if (ret)
goto ipv4;
+ sctp_csum_stub = sctp_csum_ops;
return ret;
ipv4:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH cumulus-4.1.y 1/5] vxlan: flush fdb entries on oper down
From: Stephen Hemminger @ 2017-01-23 16:59 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, netdev, ramanb, jbenc, pshelar
In-Reply-To: <1484984410-3304-1-git-send-email-roopa@cumulusnetworks.com>
On Fri, 20 Jan 2017 23:40:06 -0800
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 19b1653..15b1c23 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3276,6 +3276,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
> vxlan_handle_lowerdev_unregister(vn, dev);
> else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO)
> vxlan_push_rx_ports(dev);
> + else if (event == NETDEV_CHANGE) {
> + if (dev->netdev_ops == &vxlan_netdev_ops) {
> + if (netif_running(dev) && !netif_oper_up(dev))
> + vxlan_flush(netdev_priv(dev));
> + }
> + }
Looks correct.
Maybe logic would be clearer with a switch() statement here.
^ permalink raw reply
* Re: [PATCH cumulus-4.1.y 2/5] vxlan: don't replace fdb entry if nothing changed
From: Stephen Hemminger @ 2017-01-23 17:02 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, netdev, ramanb, jbenc, pshelar
In-Reply-To: <1484984410-3304-2-git-send-email-roopa@cumulusnetworks.com>
On Fri, 20 Jan 2017 23:40:07 -0800
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> + if (!vxlan_addr_equal(&rd->remote_ip, ip) ||
> + rd->remote_port != port ||
> + rd->remote_vni != vni ||
> + rd->remote_ifindex != ifindex) {
> + dst_cache_reset(&rd->dst_cache);
> + rd->remote_ip = *ip;
> + rd->remote_port = port;
> + rd->remote_vni = vni;
> + rd->remote_ifindex = ifindex;
> + return 1;
> + }
> +
I think it would be clearer if negative logic was avoided.
if (vxlan_addr_equal(&rd->remote_ip, ip) &&
rd->remote_port == port &&
rd->remote_vni == vni &&
rd->ermote_ifindex == ifndex)
return 1;
dst_cache_reset ...
^ permalink raw reply
* Re: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: Michael S. Tsirkin @ 2017-01-23 17:02 UTC (permalink / raw)
To: John Fastabend
Cc: Jason Wang, David Laight, john.r.fastabend@intel.com,
netdev@vger.kernel.org, alexei.starovoitov@gmail.com,
daniel@iogearbox.net
In-Reply-To: <5884319B.90909@gmail.com>
On Sat, Jan 21, 2017 at 08:14:19PM -0800, John Fastabend wrote:
> On 17-01-21 06:51 PM, Jason Wang wrote:
> >
> >
> > On 2017年01月21日 01:48, Michael S. Tsirkin wrote:
> >> On Fri, Jan 20, 2017 at 04:59:11PM +0000, David Laight wrote:
> >>> From: Michael S. Tsirkin
> >>>> Sent: 19 January 2017 21:12
> >>>>> On 2017?01?18? 23:15, Michael S. Tsirkin wrote:
> >>>>>> On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
> >>>>>>> Add support for XDP adjust head by allocating a 256B header region
> >>>>>>> that XDP programs can grow into. This is only enabled when a XDP
> >>>>>>> program is loaded.
> >>>>>>>
> >>>>>>> In order to ensure that we do not have to unwind queue headroom push
> >>>>>>> queue setup below bpf_prog_add. It reads better to do a prog ref
> >>>>>>> unwind vs another queue setup call.
> >>>>>>>
> >>>>>>> At the moment this code must do a full reset to ensure old buffers
> >>>>>>> without headroom on program add or with headroom on program removal
> >>>>>>> are not used incorrectly in the datapath. Ideally we would only
> >>>>>>> have to disable/enable the RX queues being updated but there is no
> >>>>>>> API to do this at the moment in virtio so use the big hammer. In
> >>>>>>> practice it is likely not that big of a problem as this will only
> >>>>>>> happen when XDP is enabled/disabled changing programs does not
> >>>>>>> require the reset. There is some risk that the driver may either
> >>>>>>> have an allocation failure or for some reason fail to correctly
> >>>>>>> negotiate with the underlying backend in this case the driver will
> >>>>>>> be left uninitialized. I have not seen this ever happen on my test
> >>>>>>> systems and for what its worth this same failure case can occur
> >>>>>>> from probe and other contexts in virtio framework.
> >>>>>>>
> >>>>>>> Signed-off-by: John Fastabend<john.r.fastabend@intel.com>
> >>>>>> I've been thinking about it - can't we drop
> >>>>>> old buffers without the head room which were posted before
> >>>>>> xdp attached?
> >>>>>>
> >>>>>> Avoiding the reset would be much nicer.
> >>>>>>
> >>>>>> Thoughts?
> >>>>>>
> >>>>> As been discussed before, device may use them in the same time so it's not
> >>>>> safe. Or do you mean detect them after xdp were set and drop the buffer
> >>>>> without head room, this looks sub-optimal.
> >>>>>
> >>>>> Thanks
> >>>> Yes, this is what I mean. Why is this suboptimal? It's a single branch
> >>>> in code. Yes we might lose some packets but the big hammer of device
> >>>> reset will likely lose more.
> >>> Why not leave let the hardware receive into the 'small' buffer (without
> >>> headroom) and do a copy when a frame is received.
> >>> Replace the buffers with 'big' ones for the next receive.
> >>> A data copy on a ring full of buffers won't really be noticed.
> >>>
> >>> David
> >>>
> >> I like that. John?
> >>
> >
> > This works, I prefer this only if it uses simpler code (but I suspect) than reset.
> >
> > Thanks
>
> Before the reset path I looked at doing this but it seems to require tracking
> if a buffer had headroom on a per buffer basis. I don't see a good spot to
> put a bit like this? It could go in the inbuf 'ctx' added by virtqueue_add_inbuf
> but I would need to change the current usage of ctx which in the mergeable case
> at least is just a simple pointer today. I don't like this because it
> complicates the normal path and the XDP hotpath.
>
> Otherwise we could somehow mark the ring at the point where XDP is enabled so
> that it can learn when a full iteration around the ring. But I can't see a
> simple way to make this work either.
>
> I don't know the reset look straight forward to me and although not ideal is
> fairly common on hardware based drivers during configuration changes. I'm open
> to any ideas on where to put the metadata to track headroom though.
>
> Thanks,
> John
Well with 4K pages we actually have 4 spare bits to use.
In fact this means we could reduce the mergeable buffer alignment.
It starts getting strange with 64K pages where we are
out of space, and I just noticed that with bigger pages
virtio is actually broken.
So let me fix it up first of all, and on top - maybe we can just
increase the alignment for 64k pages and up?
Truesize alignment to 512 is still reasonable and presumably
these 64k page boxes have lots of memory.
Would it make sense to tweak SK_RMEM_MAX up for larger
page sizes?
--
MST
^ permalink raw reply
* [PATCH] virtio_net: fix PAGE_SIZE > 64k
From: Michael S. Tsirkin @ 2017-01-23 17:06 UTC (permalink / raw)
To: linux-kernel; +Cc: Jason Wang, virtualization, netdev
I don't have any guests with PAGE_SIZE > 64k but the
code seems to be clearly broken in that case
as PAGE_SIZE / MERGEABLE_BUFFER_ALIGN will need
more than 8 bit and so the code in mergeable_ctx_to_buf_address
does not give us the actual true size.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Lightly tested on x86 only.
drivers/net/virtio_net.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4a10500..5e3b07e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -48,8 +48,16 @@ module_param(gso, bool, 0444);
*/
DECLARE_EWMA(pkt_len, 1, 64)
+/* With mergeable buffers we align buffer address and use the low bits to
+ * encode its true size. Buffer size is up to 1 page so we need to align to
+ * square root of page size to ensure we reserve enough bits to encode the true
+ * size.
+ */
+#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
+
/* Minimum alignment for mergeable packet buffers. */
-#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
+#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, \
+ 1U << MERGEABLE_BUFFER_MIN_ALIGN_SHIFT)
#define VIRTNET_DRIVER_VERSION "1.0.0"
--
MST
^ permalink raw reply related
* Re: [PATCH 1/3] powerpc: bpf: remove redundant check for non-null image
From: Naveen N. Rao @ 2017-01-23 17:14 UTC (permalink / raw)
To: David Miller; +Cc: ast, linuxppc-dev, daniel, netdev
In-Reply-To: <20170116.133859.1157069434816521683.davem@davemloft.net>
Hi David,
On 2017/01/16 01:38PM, David Miller wrote:
>
> I'm assuming these patches will go via the powerpc tree.
>
> If you want them to go into net-next, I kindly ask that you always
> explicitly say so, and furthermore always submit a patch series with
> a proper "[PATCH 0/N] ..." header posting.
Sure. Sorry for the confusion. I will be more explicit next time.
Thanks,
Naveen
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Oleg Nesterov @ 2017-01-23 17:18 UTC (permalink / raw)
To: Jason Baron
Cc: David S. Miller, Herbert Xu, Yauheni Kaliuta, netdev,
Eric Dumazet
In-Reply-To: <f5f9c542-2b23-9df9-098c-ad4007bf7e78@akamai.com>
On 01/23, Jason Baron wrote:
>
> >It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> >and the patch looks correct in that we need the barriers in tcp_check_space()
> >and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
> >
>
> Yes, I think it should be upgraded to an smp_mb() there. If you agree with
> this analysis, I will send a patch to upgrade it.
I think this makes sense, if nothing else to remove the obviously wrong
smp_mb__after_atomic() ;)
> Note, I did not actually
> run into this race in practice.
Yes, I am not sure it is actually necessary in practice, and even if
tcp_check_space() races with tcp_poll() and misses SOCK_NOSPACE it should
be probably called again later, but I know nothing about networking code.
We noticed this by accident, we have a bug report which really looks as a
missed wakeup in epoll, but this kernel is old and in particular it lacks
the commit 128dd1759 "epoll: prevent missed events on EPOLL_CTL_MOD" which
looks more promising. But we are not sure it can fully explain the problem
we can't reproduce, so we are looking for anything else which may contribute
to the problem.
Oleg.
^ permalink raw reply
* Re: EPOLLERR on memory mapped netlink socket
From: prashantkumar dhotre @ 2017-01-23 17:40 UTC (permalink / raw)
To: netdev
In-Reply-To: <CA+VDgmPKUnje8XTBx35kOsybwXnByUHYXwpZFjYw95V-8O1jbw@mail.gmail.com>
Appreciate any help on this
Thanks
On Mon, Jan 23, 2017 at 9:19 AM, prashantkumar dhotre
<prashantkumardhotre@gmail.com> wrote:
> Hi experts,
> I am new to netlink sockets.
> In my app , I am getting EPOLLERR in epoll_wait() on netlink socket
> continuously.
> epoll just notifies that there is a read event on socket (it does not
> tell if it is read or epollerr).
> What could be cause of this and what EPOLLERR on memory mapped netlink
> socket mean.is this Other side of netlink (kernel side) closed
> connection ?even if kernel side closed connection why non-stop
> repeated EPOLLERR s on netlink sockets ?
> What action should we take in such cases ? just close the socket or
> call getsockopt(SO_ERROR) to retrieve the pending error state from the
> socket and just continue without closing socket?
>
> how do we detect if kernel side closed the connection ?
> My understanding is :
> if we get read event notification from epoll on memory mapped netlink
> socket and in RX ring if the frame is neither NL_MMAP_STATUS_VALID and
> nor NL_MMAP_STATUS_COPY then we can conclude that this is a 'close()'
> from remote kernel socket and I can close connection by calling
> close() on my netlink socket.
> Is above understanding correct ?
>
> Please
^ permalink raw reply
* iproute2: using .maxattr = IFLA_VLAN_MAX??
From: Murali Karicheri @ 2017-01-23 17:44 UTC (permalink / raw)
To: open list:TI NETCP ETHERNET DRIVER
Hi iproute2 experts,
Found the below usage of IFLA_VLAN_MAX in ip/iplink_hsr.c
struct link_util hsr_link_util = {
.id = "hsr",
.maxattr = IFLA_VLAN_MAX, <==== should be IFLA_HSR_MAX
.parse_opt = hsr_parse_opt,
.print_opt = hsr_print_opt,
.print_help = hsr_print_help,
};
This appears to be a cut-n-paste bug as the source is based on
ip/iplink_vlan.c and should be fixed to IFLA_HSR_MAX. If yes,
I will send a patch for this.
Thanks.
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Re: [PATCH net v2] net: mpls: Fix multipath selection for LSR use case
From: David Miller @ 2017-01-23 17:48 UTC (permalink / raw)
To: dsa; +Cc: netdev, rshearma, roopa
In-Reply-To: <1484945914-32085-1-git-send-email-dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Fri, 20 Jan 2017 12:58:34 -0800
> MPLS multipath for LSR is broken -- always selecting the first nexthop
> in the one label case. For example:
>
> $ ip -f mpls ro ls
> 100
> nexthop as to 200 via inet 172.16.2.2 dev virt12
> nexthop as to 300 via inet 172.16.3.2 dev virt13
> 101
> nexthop as to 201 via inet6 2000:2::2 dev virt12
> nexthop as to 301 via inet6 2000:3::2 dev virt13
>
> In this example incoming packets have a single MPLS labels which means
> BOS bit is set. The BOS bit is passed from mpls_forward down to
> mpls_multipath_hash which never processes the hash loop because BOS is 1.
>
> Update mpls_multipath_hash to process the entire label stack. mpls_hdr_len
> tracks the total mpls header length on each pass (on pass N mpls_hdr_len
> is N * sizeof(mpls_shim_hdr)). When the label is found with the BOS set
> it verifies the skb has sufficient header for ipv4 or ipv6, and find the
> IPv4 and IPv6 header by using the last mpls_hdr pointer and adding 1 to
> advance past it.
>
> With these changes I have verified the code correctly sees the label,
> BOS, IPv4 and IPv6 addresses in the network header and icmp/tcp/udp
> traffic for ipv4 and ipv6 are distributed across the nexthops.
>
> Fixes: 1c78efa8319ca ("mpls: flow-based multipath selection")
> Acked-by: Robert Shearman <rshearma@brocade.com>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> v2
> - rebase against net/master; v1 was mistakenly based against net-next
> - updated commit message based on Robert's comment about skipping the
> first label
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] net: adaptec: starfire: add checks for dma mapping errors
From: David Miller @ 2017-01-23 17:53 UTC (permalink / raw)
To: khoroshilov; +Cc: ionut, netdev, linux-kernel, ldv-project
In-Reply-To: <1484952744-9815-1-git-send-email-khoroshilov@ispras.ru>
From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Date: Sat, 21 Jan 2017 01:52:24 +0300
> init_ring() and refill_rx_ring() don't check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
The transmit side has the same exact problem, can you take care of both
of them at the same time?
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox