* [PATCH] netfilter: fix int overflow in xt_alloc_table_info()
From: Dmitry Vyukov @ 2017-12-28 8:48 UTC (permalink / raw)
To: pablo, kadlec, fw, davem
Cc: Dmitry Vyukov, netfilter-devel, coreteam, netdev, linux-kernel
syzkaller triggered OOM kills by passing ipt_replace.size = -1
to IPT_SO_SET_REPLACE. The root cause is that SMP_ALIGN() in
xt_alloc_table_info() causes int overflow and the size check passes
when it should not. SMP_ALIGN() is no longer needed leftover.
Remove SMP_ALIGN() call in xt_alloc_table_info().
Reported-by: syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com
Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
net/netfilter/x_tables.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 55802e97f906..e02a21549c99 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -39,7 +39,6 @@ MODULE_LICENSE("GPL");
MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
MODULE_DESCRIPTION("{ip,ip6,arp,eb}_tables backend module");
-#define SMP_ALIGN(x) (((x) + SMP_CACHE_BYTES-1) & ~(SMP_CACHE_BYTES-1))
#define XT_PCPU_BLOCK_SIZE 4096
struct compat_delta {
@@ -1000,7 +999,7 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
return NULL;
/* Pedantry: prevent them from hitting BUG() in vmalloc.c --RR */
- if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
+ if ((size >> PAGE_SHIFT) + 2 > totalram_pages)
return NULL;
info = kvmalloc(sz, GFP_KERNEL);
--
2.15.1.620.gb9897f4670-goog
^ permalink raw reply related
* Re: lost connection to test machine (3)
From: Dmitry Vyukov @ 2017-12-28 8:50 UTC (permalink / raw)
To: Florian Westphal
Cc: syzbot, LKML, syzkaller-bugs, Pablo Neira Ayuso, Jozsef Kadlecsik,
David Miller, netfilter-devel, coreteam, netdev
In-Reply-To: <20171227213627.GC23214@breakpoint.cc>
On Wed, Dec 27, 2017 at 10:36 PM, Florian Westphal <fw@strlen.de> wrote:
> Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Wed, Dec 27, 2017 at 7:18 PM, syzbot
>> <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com> wrote:
>> > Hello,
>> >
>> > syzkaller hit the following crash on
>> > beacbc68ac3e23821a681adb30b45dc55b17488d
>> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>> > compiler: gcc (GCC) 7.1.1 20170620
>> > .config is attached
>> > Raw console output is attached.
>> > C reproducer is attached
>> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> > for information about syzkaller reproducers
>> >
>> >
>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > Reported-by: <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com>
>> > It will help syzbot understand when the bug is fixed. See footer for
>> > details.
>> > If you forward the report, please keep this part and the footer.
>>
>> +netfilter maintainers
>>
>> Here is cleaned reproducer:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <sys/types.h>
>> #include <sys/socket.h>
>> #include <netinet/in.h>
>> #include <netinet/tcp.h>
>> #include <linux/if.h>
>> #include <linux/netfilter_ipv4/ip_tables.h>
>>
>> int main()
>> {
>> int fd;
>>
>> fd = socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
>> struct ipt_replace opt = {};
>> opt.num_counters = 1;
>> opt.size = -1;
>> setsockopt(fd, SOL_IP, 0x40, &opt, 0x4);
>> return 0;
>> }
>>
>>
>> What happens there is that here:
>>
>> struct xt_table_info *xt_alloc_table_info(unsigned int size)
>> {
>> ...
>> if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
>> return NULL;
>>
>> size = -1 and SMP_ALIGN(size) = 0, so this still tries to allocate
>> 4GB+delta bytes.
>>
>> I don't understand why this uses SMP_ALIGN since we add 2 pages on
>> top, it seems that we could just drop SMP_ALIGN and local SMP_ALIGN
>> definition altogether.
>
> Looking at history.git this seems to be a left over from back when
> iptables allocated size * num_cpus() (and used an SMP_ALIGN based offset
> for each cpu).
>
> So yes, I think we can just toss/drop this.
Thanks.
I've mailed "netfilter: fix int overflow in xt_alloc_table_info()" to fix this.
^ permalink raw reply
* Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface
From: Antoine Tenart @ 2017-12-28 10:04 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, davem, kishon, andrew, jason,
sebastian.hesselbarth, gregory.clement, mw, stefanc, ymarkman,
thomas.petazzoni, miquel.raynal, nadavh, netdev, linux-arm-kernel,
linux-kernel, Jon Nettleton
In-Reply-To: <20171227231959.GU10595@n2100.armlinux.org.uk>
Hi Russell,
On Wed, Dec 27, 2017 at 11:20:00PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> >
> > What do you suggest to describe this in the dt, to enable a port using
> > the current PPv2 driver?
>
> I don't - I'm merely pointing out that you're bodging support for the
> SFP cage rather than productively discussing phylink for mvpp2.
>
> As far as I remember, the discussion stalled at this point:
>
> - You think there's modes that mvpp2 supports that are not supportable
> if you use phylink.
>
> - I've described what phylink supports, and I've asked you for details
> about what you can't support.
That's not what I remembered. You had some valid points, and others
related to PHY modes the driver wasn't supporting before the phylink
transition. My understanding of this was that you wanted a full
featured support while I only wanted to convert the already supported
modes.
> Unfortunately, no details have been forthcoming, and no further
> discussion has occurred - the ball is entirely in your court to
> progress this issue since I requested information from you and that
> is where things seem to have stalled.
>
> The result is that, with your patch, you're locking the port to 2.5G
> speeds, meaning that only 4.3Mbps Fibrechannel SFPs can be used with
> the port, and it can only be used with another device that supports
> 2.5G speeds. You can't use a copper RJ45 module, and you can't use
> a standard 1000base-X module either in this configuration.
You're probably right about not wanting this dt patch. The non-dt
patches still are relevant regardless of phylink being supported in the
PPv2 driver. I'll send a v2 without the dt parts.
Regarding the phylink patch it's stalled for now as I have other
priorities, but I do agree it's a topic that needs to be worked on for a
proper support. I initially made a patch to be nice as it was mentioned
on a previous series, but given the feedback I got I decided to delay it
until my other tasks were completed.
So let's delay the fourth interface support on the mcbin for now.
> What I'm most concerned about, given the bindings for comphy that
> have been merged, is that Free Electrons is pushing forward seemingly
> with no regard to the requirement that the serdes lanes are dynamically
> reconfigurable, and that's a basic requirement for SFP, and for the
> 88x3310 PHYs on the Macchiatobin platform.
The main idea behind the comphy driver is to provide a way to
reconfigure the serdes lanes at runtime. Could you develop what are
blocking points to properly support SFP, regarding the current comphy
support?
Thanks,
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface
From: Antoine Tenart @ 2017-12-28 10:05 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King - ARM Linux, Antoine Tenart, davem, kishon, jason,
sebastian.hesselbarth, gregory.clement, mw, stefanc, ymarkman,
thomas.petazzoni, miquel.raynal, nadavh, netdev, linux-arm-kernel,
linux-kernel
In-Reply-To: <20171228074623.GA28444@lunn.ch>
Hi Andrew,
On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >
> > > +&cps_eth2 {
> > > + /* CPS Lane 5 */
> > > + status = "okay";
> > > + phy-mode = "2500base-x";
> > > + /* Generic PHY, providing serdes lanes */
> > > + phys = <&cps_comphy5 2>;
> > > +};
> > > +
> >
> > This is wrong. This lane is connected to a SFP cage which can support
> > more than just 2500base-X. Tying it in this way to 2500base-X means
> > that this port does not support conenctions at 1000base-X, despite
> > that's one of the most popular and more standardised speeds.
> >
>
> I agree with Russell here. SFP modules are hot pluggable, and support
> a range of interface modes. You need to query what the SFP module is
> in order to know how to configure the SERDES interface. The phylink
> infrastructure does that for you.
Sure, I understand. We'll be able to support such interfaces only when
the phylink PPv2 support lands in.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH net-next 1/6] phy: add 2.5G SGMII mode to the phy_mode enum
From: Antoine Tenart @ 2017-12-28 10:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, kishon, jason, sebastian.hesselbarth,
gregory.clement, linux, mw, stefanc, ymarkman, thomas.petazzoni,
miquel.raynal, nadavh, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20171228072053.GA27336@lunn.ch>
Hi Andrew,
On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> > This patch adds one more generic PHY mode to the phy_mode enum, to allow
> > configuring generic PHYs to the 2.5G SGMII mode by using the set_mode
> > callback.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> > include/linux/phy/phy.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 4f8423a948d5..70459a28f3a1 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -28,6 +28,7 @@ enum phy_mode {
> > PHY_MODE_USB_DEVICE,
> > PHY_MODE_USB_OTG,
> > PHY_MODE_SGMII,
> > + PHY_MODE_SGMII_2_5G,
> > PHY_MODE_10GKR,
> > PHY_MODE_UFS_HS_A,
> > PHY_MODE_UFS_HS_B,
>
> There was a discussion maybe last month about adding 2.5G SGMII. I
> would prefer 2500SGMII. Putting the number first makes it uniform with
> the other defines, 1000BASEX, 25000BASEX, 10GKR.
Good to know. I wasn't completely sure how to name this mode properly,
but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
v2 (without the dt part).
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH iproute2 1/3] vxcan,veth: Forbid "type" for peer device
From: Serhey Popovych @ 2017-12-28 10:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171226090535.51ac43ef@xeon-e3>
[-- Attachment #1.1: Type: text/plain, Size: 1170 bytes --]
Stephen Hemminger wrote:
> On Wed, 20 Dec 2017 09:37:29 +0200
> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>
>> It is already given for original device we configure this
>> peer for.
>>
>> Results from following command before/after change applied
>> are shown below:
>>
>> $ ip link add dev veth1a type veth peer name veth1b \
>> type veth peer name veth1c
>>
>> Before:
>> -------
>>
>> <no output, no netdevs created>
>>
>> After:
>> ------
>>
>> Error: argument "type" is wrong: not supported for peer
>>
>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>
> This makes a lot of sense.
> Maybe the better error message would be to use duparg() since
> that is the most likely cause of the user error.
Not sure Stephen. With duparg() I get following error message for
test case described in above.
Error: duplicate "type": "veth" is the second value.
It looks good for me, but we have two "type veth" in command and
which one is wrong might not be very clear to user.
Anyway I send v2 for this particular change to get and option
to pick the best one.
Thanks.
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* [PATCH iproute2 1/3] vxcan,veth: Forbid "type" for peer device
From: Serhey Popovych @ 2017-12-28 11:01 UTC (permalink / raw)
To: netdev
In-Reply-To: <20171226090535.51ac43ef@xeon-e3>
It is already given for original device we configure this
peer for.
Results from following command before/after change applied
are shown below:
$ ip link add dev veth1a type veth peer name veth1b \
type veth peer name veth1c
Before:
-------
<no output, no netdevs created>
After:
------
Error: duplicate "type": "veth" is the second value.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/iplink_vxcan.c | 3 +++
ip/link_veth.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index c13224c..ed0ad8b 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -65,6 +65,9 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
if (err < 0)
return err;
+ if (type)
+ duparg("type", argv[err]);
+
if (name) {
addattr_l(hdr, 1024,
IFLA_IFNAME, name, strlen(name) + 1);
diff --git a/ip/link_veth.c b/ip/link_veth.c
index fcfd1ef..fddb7ac 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -63,6 +63,9 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
if (err < 0)
return err;
+ if (type)
+ duparg("type", argv[err]);
+
if (name) {
addattr_l(hdr, 1024,
IFLA_IFNAME, name, strlen(name) + 1);
--
1.7.10.4
^ permalink raw reply related
* [PATCH net v1 1/1] tipc: fix hanging poll() for stream sockets
From: Parthasarathy Bhuvaragan @ 2017-12-28 11:03 UTC (permalink / raw)
To: netdev; +Cc: tipc-discussion, jon.maloy, Parthasarathy Bhuvaragan
In commit 42b531de17d2f6 ("tipc: Fix missing connection request
handling"), we replaced unconditional wakeup() with condtional
wakeup for clients with flags POLLIN | POLLRDNORM | POLLRDBAND.
This breaks the applications which do a connect followed by poll
with POLLOUT flag. These applications are not woken when the
connection is ESTABLISHED and hence sleep forever.
In this commit, we fix it by including the POLLOUT event for
sockets in TIPC_CONNECTING state.
Fixes: 42b531de17d2f6 ("tipc: Fix missing connection request handling")
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@gmail.com>
---
net/tipc/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 41127d0b925e..3b4084480377 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -727,11 +727,11 @@ static unsigned int tipc_poll(struct file *file, struct socket *sock,
switch (sk->sk_state) {
case TIPC_ESTABLISHED:
+ case TIPC_CONNECTING:
if (!tsk->cong_link_cnt && !tsk_conn_cong(tsk))
revents |= POLLOUT;
/* fall thru' */
case TIPC_LISTEN:
- case TIPC_CONNECTING:
if (!skb_queue_empty(&sk->sk_receive_queue))
revents |= POLLIN | POLLRDNORM;
break;
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2 3/3] ip/tunnel: Document "external" parameter
From: Serhey Popovych @ 2017-12-28 11:11 UTC (permalink / raw)
To: netdev
In-Reply-To: <1514374096-1473-4-git-send-email-serhe.popovych@gmail.com>
Add it to ip-link(8) "type gre" output help message
as well as to ip-link(8) page.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 1 +
man/man8/ip-link.8.in | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 896bb19..3c0b6d6 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -43,6 +43,7 @@ static void print_usage(FILE *f)
" [ [no]encap-csum ]\n"
" [ [no]encap-csum6 ]\n"
" [ [no]encap-remcsum ]\n"
+ " [ external ]\n"
" [ fwmark MARK ]\n"
" [ erspan IDX ]\n"
"\n"
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 94ecbec..9ddf0ff 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -698,6 +698,8 @@ the following additional arguments are supported:
.I " mode " { ip6ip | ipip | mplsip | any } "
] [
.BR erspan " \fIIDX "
+] [
+.BR external
]
.in +8
@@ -749,6 +751,11 @@ IPv6-Over-IPv4 is not supported for IPIP.
indicates a 20 bit index/port number associated with the ERSPAN
traffic's source port and direction.
+.sp
+.BR external
+- make this tunnel externally controlled
+.RB "(e.g. " "ip route encap" ).
+
.in -8
.TP
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2] gre/tunnel: Print erspan_index using print_uint()
From: Serhey Popovych @ 2017-12-28 11:12 UTC (permalink / raw)
To: netdev
One is missing in JSON output because fprintf()
is used instead of print_uint().
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 3 ++-
ip/link_gre6.c | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 896bb19..1e331c8 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -476,7 +476,8 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (tb[IFLA_GRE_ERSPAN_INDEX]) {
__u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
- fprintf(f, "erspan_index %u ", erspan_idx);
+ print_uint(PRINT_ANY,
+ "erspan_index", "erspan_index %u ", erspan_idx);
}
if (tb[IFLA_GRE_ENCAP_TYPE] &&
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..2687a62 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -532,7 +532,9 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (tb[IFLA_GRE_ERSPAN_INDEX]) {
__u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
- fprintf(f, "erspan_index %u ", erspan_idx);
+
+ print_uint(PRINT_ANY,
+ "erspan_index", "erspan_index %u ", erspan_idx);
}
if (tb[IFLA_GRE_ENCAP_TYPE] &&
--
1.7.10.4
^ permalink raw reply related
* [RFC] Supporting namespaces in the connector driver
From: Elad Wexler @ 2017-12-28 12:05 UTC (permalink / raw)
To: netdev, linux-kernel, zbr
Hi,
I have spent some time looking at the cn_proc monitor capabilities
which use the connector driver for monitoring processes.
Currently the cn_proc (and all other modules which are using the connector)
only works on the main init_user_ns namespace.
If I am working inside a docker container (as an example), I won't be able to monitor process
creation, for example fork()/exec() etc ...
Of course I will be able to monitor any process from the host namespace (init_user_ns)
but I would like also to be able to monitor the processes that belong to the same
docker container, (belong to the same namespace)
I wonder if there is a plan to add pernet support for the connector?
If you think it there is a good reason to add that? (I can provide a few patches that do that)
I will be happy to work on to add a support for that, and actually I have started to
do some modification to make it works. and still testing it.
Thanks
^ permalink raw reply
* Re: [PATCH 1/3] net: Fix possible race in peernet2id_alloc()
From: Kirill Tkhai @ 2017-12-28 12:55 UTC (permalink / raw)
To: Eric W. Biederman, davem; +Cc: netdev, eric.dumazet
In-Reply-To: <54b5eb20-0357-d0f2-75ef-3de093c3a283@virtuozzo.com>
Hi, David,
I see the status of patchset has changed on https://patchwork.ozlabs.org/patch/851928/
and now it is "Changes Requested".
There was Eric's reply, and my reply on his message. After that there wasn't objections on
my reply or other patches.
Could you please clarify the status or what I should do with the patchset
(because it's not clear for me)?
Thanks,
Kirill
On 22.12.2017 13:30, Kirill Tkhai wrote:
> Hi, Eric,
>
> On 21.12.2017 20:39, Eric W. Biederman wrote:
>> Kirill Tkhai <ktkhai@virtuozzo.com> writes:
>>
>>> peernet2id_alloc() is racy without rtnl_lock() as atomic_read(&peer->count)
>>> under net->nsid_lock does not guarantee, peer is alive:
>>>
>>> rcu_read_lock()
>>> peernet2id_alloc() ..
>>> spin_lock_bh(&net->nsid_lock) ..
>>> atomic_read(&peer->count) == 1 ..
>>> .. put_net()
>>> .. cleanup_net()
>>> .. for_each_net(tmp)
>>> .. spin_lock_bh(&tmp->nsid_lock)
>>> .. __peernet2id(tmp, net) == -1
>>> .. ..
>>> .. ..
>>> __peernet2id_alloc(alloc == true) ..
>>> .. ..
>>> rcu_read_unlock() ..
>>> .. synchronize_rcu()
>>> .. kmem_cache_free(net)
>>>
>>> After the above situation, net::netns_id contains id pointing to freed memory,
>>> and any other dereferencing by the id will operate with this freed memory.
>>>
>>> Currently, peernet2id_alloc() is used under rtnl_lock() everywhere except
>>> ovs_vport_cmd_fill_info(), and this race can't occur. But peernet2id_alloc()
>>> is generic interface, and better we fix it before someone really starts
>>> use it in wrong context.
>>
>> So it comes down to this piece of code from ovs and just let me say ick.
>> if (!net_eq(net, dev_net(vport->dev))) {
>> int id = peernet2id_alloc(net, dev_net(vport->dev));
>>
>> if (nla_put_s32(skb, OVS_VPORT_ATTR_NETNSID, id))
>> goto nla_put_failure;
>> }
>>
>> Without the rtnl lock dev_net can cange between the test and the
>> call of peernet2id_alloc.
>
> The places like this are usually protected via netdevice notifier. There is
> ovs_dp_device_notifier in openvswitch and it seems to be a handler for
> the situation above. But I'm not sure it's safe, and it's not clear for
> me why it does not take ovl_mutex to synchronize with ovs_vport_cmd_fill_info(),
> and whether there is another synchronization.
>
>> At first glance it looks like the bug is that we are running a control
>> path of the networking stack without the rtnl lock. So it may be that
>> ASSERT_RTNL() is the better fix.
>
> Do you mean inserting ASSERT_RTNL() into peernet2id_alloc()?
> If so, it's not need, as all nsid are protected via separate lock starting from
>
> commit 95f38411df05 "netns: use a spin_lock to protect nsid management"
>
> and commit de133464c9e7 "netns: make nsid_lock per net".
>
> nsid primitives are aimed to be safe without respect to rtnl_lock().
> My patch just adds some sanity checks to make them more safe.
>
>> Given that it would be nice to reduce the scope of the rtnl lock this
>> might not be a bad direction. Let me see.
>>> Is rtnl_notify safe without the rtnl lock?
>
> It seems they have to be safe, as there is only sending a skb to netlink.
> rtnl_notify() is used without rtnl_lock() in most other places of net code.
>
> The only reason rtnl_lock() is needed in cleanup_net() is we unlink net from the list.
> rtnl_lock() around nsid cleanup is leftover after nsid are made protected via
> separate lock. So we may relax rtnl_lock() in cleanup_net() and I'm going to do that
> in one of next patches (not yet sent).
>
>>>
>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>> ---
>>> net/core/net_namespace.c | 23 +++++++++++++++++++----
>>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>>> index 60a71be75aea..6a4eab438221 100644
>>> --- a/net/core/net_namespace.c
>>> +++ b/net/core/net_namespace.c
>>> @@ -221,17 +221,32 @@ static void rtnl_net_notifyid(struct net *net, int cmd, int id);
>>> */
>>> int peernet2id_alloc(struct net *net, struct net *peer)
>>> {
>>> - bool alloc;
>>> + bool alloc = false, alive = false;
>>> int id;
>>
>> ^^^ Perhaps we want "ASSERT_RTNL();" here?
>>>
>>> - if (atomic_read(&net->count) == 0)
>>> - return NETNSA_NSID_NOT_ASSIGNED;
>>
>> Moving this hunk is of no benefit. The code must be called with a valid
>> reference to net. Which means net->count is a fancy way of testing to
>> see if the code is in cleanup_net. In all other cases net->count should
>> be non-zero and it should remain that way because of our caller must
>> keep a reference.
>
> Let me discuss about this.
> This interface is exported to drivers, and they don't always follow the rules
> about what is allowed or disallowed. This hunk adds sanity checks, which
> protects us from bad drivers code etc. Some people want to use peernet2id_alloc()
> in generic functions, which may be called in different context (also, for net
> obtained from rcu list). Instead of every user makes such a test in its function,
> this hunks introduces generic valid check, which allows people do not insert n+1
> their own checks.
>
> Also, unlocked check does not give performance advantages as later we take the lock
> *almost always*. So, my suggestion is to make peernet2id_alloc() more safe for
> end user.
>
>>> spin_lock_bh(&net->nsid_lock);
>>> - alloc = atomic_read(&peer->count) == 0 ? false : true;
>>> + /* Spinlock guarantees we never hash a peer to net->netns_ids
>>> + * after idr_destroy(&net->netns_ids) occurs in cleanup_net().
>>> + */
>>> + if (atomic_read(&net->count) == 0) {
>>> + id = NETNSA_NSID_NOT_ASSIGNED;
>>> + goto unlock;
>>> + }
>>> + /*
>>> + * When peer is obtained from RCU lists, we may race with
>>> + * its cleanup. Check whether it's alive, and this guarantees
>>> + * we never hash a peer back to net->netns_ids, after it has
>>> + * just been idr_remove()'d from there in cleanup_net().
>>> + */
>>> + if (maybe_get_net(peer))
>>> + alive = alloc = true;
>>
>> Yes this does seem reasonable. The more obvious looking code which
>> would return NETNSA_NSID_NOT_ASSIGNED if the peer has a count of 0, is
>> silly as it makes would make it appear that a peer is momentary outside
>> of a network namespace when the peer is in fact moving from one network
>> namespace to another.
>>
>>> id = __peernet2id_alloc(net, peer, &alloc);
>>> +unlock:
>>> spin_unlock_bh(&net->nsid_lock);
>>> if (alloc && id >= 0)
>>> rtnl_net_notifyid(net, RTM_NEWNSID, id);
>> ^^^^^^
>> Is this safe without the rtnl lock?
>
> Yes, it seems to be safe. Please, look above.
>
>>> + if (alive)
>>> + put_net(peer);
>>> return id;
>>> }
>>> EXPORT_SYMBOL_GPL(peernet2id_alloc);
>
> Kirill
>
^ permalink raw reply
* Re: INFO: task hung in cleanup_net
From: Dmitry Vyukov @ 2017-12-28 13:56 UTC (permalink / raw)
To: David Ahern
Cc: syzbot, LKML, Ingo Molnar, Peter Zijlstra, syzkaller-bugs,
David Miller, Florian Westphal, Daniel Borkmann, Xin Long,
jakub.kicinski, mschiffer, Vladislav Yasevich, Jiri Benc, netdev
In-Reply-To: <c41d642e-71ea-422e-2d6d-048a83e45143@gmail.com>
On Tue, Dec 19, 2017 at 7:17 PM, David Ahern <dsahern@gmail.com> wrote:
> On 12/19/17 5:47 AM, Dmitry Vyukov wrote:
>> On Tue, Dec 19, 2017 at 1:33 PM, syzbot
>> <bot+b17f10c8a8c693b40723d40d6553fbc54d197679@syzkaller.appspotmail.com>
>> wrote:
>>> Hello,
>>>
>>> syzkaller hit the following crash on
>>> e40fd8d6b4d9f59b160faa1736f78fc07533ff37
>>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
>>> compiler: gcc (GCC) 7.1.1 20170620
>>> .config is attached
>>> Raw console output is attached.
>>>
>>> Unfortunately, I don't have any reproducer for this bug yet.
>>>
>>>
>>> sctp: sctp_transport_update_pmtu: Reported pmtu 508 too low, using default
>>> minimum of 512
>>> sctp: sctp_transport_update_pmtu: Reported pmtu 508 too low, using default
>>> minimum of 512
>>> sctp: sctp_transport_update_pmtu: Reported pmtu 508 too low, using default
>>> minimum of 512
>>> INFO: task kworker/u4:11:3785 blocked for more than 120 seconds.
>>> Not tainted 4.15.0-rc2-next-20171207+ #61
>>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>>> kworker/u4:11 D16448 3785 2 0x80000000
>>> Workqueue: netns cleanup_net
>>> Call Trace:
>>> context_switch kernel/sched/core.c:2800 [inline]
>>> __schedule+0x8eb/0x2060 kernel/sched/core.c:3376
>>> schedule+0xf5/0x430 kernel/sched/core.c:3435
>>> schedule_preempt_disabled+0x10/0x20 kernel/sched/core.c:3493
>>> __mutex_lock_common kernel/locking/mutex.c:833 [inline]
>>> __mutex_lock+0xaad/0x1a80 kernel/locking/mutex.c:893
>>> mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:908
>>> rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74
>>> cleanup_net+0x24c/0xb60 net/core/net_namespace.c:453
>>> process_one_work+0xbfd/0x1bc0 kernel/workqueue.c:2113
>>> worker_thread+0x223/0x1990 kernel/workqueue.c:2247
>>> kthread+0x37a/0x440 kernel/kthread.c:238
>>> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:524
>>>
>>> Showing all locks held in the system:
>>> 2 locks held by khungtaskd/672:
>>> #0: (rcu_read_lock){....}, at: [<00000000b9bef8ab>]
>>> check_hung_uninterruptible_tasks kernel/hung_task.c:175 [inline]
>>> #0: (rcu_read_lock){....}, at: [<00000000b9bef8ab>] watchdog+0x1c5/0xd60
>>> kernel/hung_task.c:249
>>> #1: (tasklist_lock){.+.+}, at: [<000000006d6acf8b>]
>>> debug_show_all_locks+0xd3/0x400 kernel/locking/lockdep.c:4554
>>> 1 lock held by rsyslogd/2970:
>>> #0: (&f->f_pos_lock){+.+.}, at: [<000000001cede688>]
>>> __fdget_pos+0x131/0x1a0 fs/file.c:765
>>> 2 locks held by getty/3052:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3053:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3054:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3055:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3056:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3057:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 2 locks held by getty/3058:
>>> #0: (&tty->ldisc_sem){++++}, at: [<000000004e0245ba>]
>>> ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
>>> #1: (&ldata->atomic_read_lock){+.+.}, at: [<00000000ff863b18>]
>>> n_tty_read+0x2f2/0x1a10 drivers/tty/n_tty.c:2131
>>> 1 lock held by syz-executor2/3320:
>>> #0: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>> 4 locks held by kworker/u4:11/3785:
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> __write_once_size include/linux/compiler.h:212 [inline]
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> atomic_long_set include/asm-generic/atomic-long.h:57 [inline]
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> set_work_data kernel/workqueue.c:619 [inline]
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> set_work_pool_and_clear_pending kernel/workqueue.c:646 [inline]
>>> #0: ((wq_completion)"%s""netns"){+.+.}, at: [<0000000087d81703>]
>>> process_one_work+0xad4/0x1bc0 kernel/workqueue.c:2084
>>> #1: (net_cleanup_work){+.+.}, at: [<000000008a817b5c>]
>>> process_one_work+0xb2f/0x1bc0 kernel/workqueue.c:2088
>>> #2: (net_mutex){+.+.}, at: [<0000000079150fd8>] cleanup_net+0x247/0xb60
>>> net/core/net_namespace.c:450
>>> #3: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>> 3 locks held by kworker/0:6/14978:
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> __write_once_size include/linux/compiler.h:212 [inline]
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> atomic_long_set include/asm-generic/atomic-long.h:57 [inline]
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> set_work_data kernel/workqueue.c:619 [inline]
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> set_work_pool_and_clear_pending kernel/workqueue.c:646 [inline]
>>> #0: ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at: [<0000000087d81703>]
>>> process_one_work+0xad4/0x1bc0 kernel/workqueue.c:2084
>>> #1: ((addr_chk_work).work){+.+.}, at: [<000000008a817b5c>]
>>> process_one_work+0xb2f/0x1bc0 kernel/workqueue.c:2088
>>> #2: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>> 1 lock held by syz-executor2/15852:
>>> #0: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>> 1 lock held by syz-executor7/15958:
>>> #0: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>> 1 lock held by syz-executor7/15961:
>>> #0: (rtnl_mutex){+.+.}, at: [<0000000063c47ced>] rtnl_lock+0x17/0x20
>>> net/core/rtnetlink.c:74
>>>
>>> =============================================
>>>
>>> NMI backtrace for cpu 1
>>> CPU: 1 PID: 672 Comm: khungtaskd Not tainted 4.15.0-rc2-next-20171207+ #61
>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>> Google 01/01/2011
>>> Call Trace:
>>> __dump_stack lib/dump_stack.c:17 [inline]
>>> dump_stack+0x194/0x257 lib/dump_stack.c:53
>>> nmi_cpu_backtrace+0x1d2/0x210 lib/nmi_backtrace.c:103
>>> nmi_trigger_cpumask_backtrace+0x122/0x180 lib/nmi_backtrace.c:62
>>> arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
>>> trigger_all_cpu_backtrace include/linux/nmi.h:138 [inline]
>>> check_hung_task kernel/hung_task.c:132 [inline]
>>> check_hung_uninterruptible_tasks kernel/hung_task.c:190 [inline]
>>> watchdog+0x90c/0xd60 kernel/hung_task.c:249
>>> kthread+0x37a/0x440 kernel/kthread.c:238
>>> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:524
>>> Sending NMI from CPU 1 to CPUs 0:
>>> NMI backtrace for cpu 0
>>> CPU: 0 PID: 15938 Comm: syz-executor7 Not tainted 4.15.0-rc2-next-20171207+
>>> #61
>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>> Google 01/01/2011
>>> RIP: 0010:inb arch/x86/include/asm/io.h:348 [inline]
>>> RIP: 0010:io_serial_in+0x6b/0x90 drivers/tty/serial/8250/8250_port.c:434
>>> RSP: 0018:ffff8801c1c173e8 EFLAGS: 00000002
>>> RAX: dffffc0000000000 RBX: 00000000000003fd RCX: 0000000000000000
>>> RDX: 00000000000003fd RSI: ffffc90001c88000 RDI: ffffffff880fa9c0
>>> RBP: ffff8801c1c173f8 R08: 0000000000000002 R09: 000000000000000c
>>> R10: 0000000000000000 R11: ffffffff87896d20 R12: ffffffff880fa980
>>> R13: 0000000000000020 R14: fffffbfff101f577 R15: fffffbfff101f53a
>>> FS: 00007f1ef8fad700(0000) GS:ffff8801db200000(0000) knlGS:0000000000000000
>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: 00007fbea91b6000 CR3: 00000001d0aaa000 CR4: 00000000001406f0
>>> DR0: 0000000020001000 DR1: 0000000020001000 DR2: 0000000020001020
>>> DR3: 0000000020000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
>>> Call Trace:
>>> serial_in drivers/tty/serial/8250/8250.h:111 [inline]
>>> wait_for_xmitr+0x93/0x1e0 drivers/tty/serial/8250/8250_port.c:2033
>>> serial8250_console_putchar+0x1f/0x60
>>> drivers/tty/serial/8250/8250_port.c:3170
>>> uart_console_write+0xac/0xe0 drivers/tty/serial/serial_core.c:1858
>>> serial8250_console_write+0x647/0xa20
>>> drivers/tty/serial/8250/8250_port.c:3236
>>> univ8250_console_write+0x5f/0x70 drivers/tty/serial/8250/8250_core.c:590
>>> call_console_drivers kernel/printk/printk.c:1574 [inline]
>>> console_unlock+0x788/0xd70 kernel/printk/printk.c:2233
>>> vprintk_emit+0x4ad/0x590 kernel/printk/printk.c:1757
>>> vprintk_default+0x28/0x30 kernel/printk/printk.c:1796
>>> vprintk_func+0x57/0xc0 kernel/printk/printk_safe.c:379
>>> printk+0xaa/0xca kernel/printk/printk.c:1829
>>> __dev_set_promiscuity+0x2a4/0x630 net/core/dev.c:6609
>>> __dev_change_flags+0x559/0x990 net/core/dev.c:6826
>>> dev_change_flags+0x88/0x140 net/core/dev.c:6886
>>> dev_ifsioc+0x60d/0x9b0 net/core/dev_ioctl.c:257
>>> dev_ioctl+0x2c2/0xf90 net/core/dev_ioctl.c:566
>>> sock_do_ioctl+0x94/0xb0 net/socket.c:971
>>> sock_ioctl+0x2c2/0x440 net/socket.c:1061
>>> vfs_ioctl fs/ioctl.c:46 [inline]
>>> do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:686
>>> SYSC_ioctl fs/ioctl.c:701 [inline]
>>> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
>>> entry_SYSCALL_64_fastpath+0x1f/0x96
>>> RIP: 0033:0x452a39
>>> RSP: 002b:00007f1ef8facc58 EFLAGS: 00000212 ORIG_RAX: 0000000000000010
>>> RAX: ffffffffffffffda RBX: 00007f1ef8fad700 RCX: 0000000000452a39
>>> RDX: 00000000208a3fe0 RSI: 0000000000008914 RDI: 0000000000000019
>>> RBP: 0000000000a6f880 R08: 0000000000000000 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000212 R12: 0000000000000000
>>> R13: 0000000000a6f7ff R14: 00007f1ef8fad9c0 R15: 0000000000000002
>>> Code: 24 d9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa
>>> 48 c1 ea 03 d3 e3 80 3c 02 00 75 17 41 03 5c 24 40 89 da ec <5b> 0f b6 c0 41
>>> 5c 5d c3 e8 88 ad 18 ff eb c2 e8 e1 ad 18 ff eb
>>
>>
>> This looks like +rtnetlink issue.
>
> Perhaps related to / fixed by: http://patchwork.ozlabs.org/patch/850957/
If you think so, let's tell syzbot about it:
#syz fix: net: Fix double free and memory corruption in get_net_ns_by_id()
^ permalink raw reply
* Re: [PATCH net-next 1/6] phy: add 2.5G SGMII mode to the phy_mode enum
From: Florian Fainelli @ 2017-12-28 14:16 UTC (permalink / raw)
To: Antoine Tenart, Andrew Lunn
Cc: thomas.petazzoni, ymarkman, jason, netdev, linux-kernel, linux,
kishon, nadavh, miquel.raynal, gregory.clement, stefanc, mw,
davem, linux-arm-kernel, sebastian.hesselbarth
In-Reply-To: <20171228100656.GF2626@kwain>
On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> Hi Andrew,
>
> On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
>>> This patch adds one more generic PHY mode to the phy_mode enum, to allow
>>> configuring generic PHYs to the 2.5G SGMII mode by using the set_mode
>>> callback.
>>>
>>> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>>> ---
>>> include/linux/phy/phy.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>> index 4f8423a948d5..70459a28f3a1 100644
>>> --- a/include/linux/phy/phy.h
>>> +++ b/include/linux/phy/phy.h
>>> @@ -28,6 +28,7 @@ enum phy_mode {
>>> PHY_MODE_USB_DEVICE,
>>> PHY_MODE_USB_OTG,
>>> PHY_MODE_SGMII,
>>> + PHY_MODE_SGMII_2_5G,
>>> PHY_MODE_10GKR,
>>> PHY_MODE_UFS_HS_A,
>>> PHY_MODE_UFS_HS_B,
>>
>> There was a discussion maybe last month about adding 2.5G SGMII. I
>> would prefer 2500SGMII. Putting the number first makes it uniform with
>> the other defines, 1000BASEX, 25000BASEX, 10GKR.
>
> Good to know. I wasn't completely sure how to name this mode properly,
> but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> v2 (without the dt part).
And since you are respinning, please make sure you update phy_modes() in
the same header file as well as
Documentation/devicetree/bindings/net/ethernet.txt with the newly added
PHY interface mode.
--
Florian
^ permalink raw reply
* [PATCH net-next 0/4] mlx4 misc for 4.16
From: Tariq Toukan @ 2017-12-28 14:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
Hi Dave,
This patchset contains misc cleanups and improvements
to the mlx4 Core and Eth drivers.
In patches 1 and 2 I reduce and reorder the branches in the RX csum flow.
In patch 3 I align the FMR unmapping flow with the device spec, to allow
a remapping afterwards.
Patch 4 by Moni changes the default QoS settings so that a pause
frame stops all traffic regardless of its prio.
Series generated against net-next commit:
836df24a7062 net: hns3: hns3_get_channels() can be static
Thanks,
Tariq.
Moni Shoua (1):
net/mlx4_en: Change default QoS settings
Tariq Toukan (3):
net/mlx4_en: RX csum, remove redundant branches and checks
net/mlx4_en: RX csum, reorder branches
net/mlx4_core: Cleanup FMR unmapping flow
drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c | 5 +++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 7 ++++
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 56 +++++++++++++-------------
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 1 +
drivers/net/ethernet/mellanox/mlx4/mr.c | 40 +++++++++---------
5 files changed, 63 insertions(+), 46 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 2/4] net/mlx4_en: RX csum, reorder branches
From: Tariq Toukan @ 2017-12-28 14:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <1514471171-3894-1-git-send-email-tariqt@mellanox.com>
Use early goto commands, and save else branches.
This uses less indentations and brackets, making the code
more readable.
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 46 ++++++++++++++----------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 680bd3fbaa60..5f9dbc9a7f5b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -816,37 +816,33 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
if (likely(dev->features & NETIF_F_RXCSUM)) {
if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP |
MLX4_CQE_STATUS_UDP)) {
- if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&
- cqe->checksum == cpu_to_be16(0xffff)) {
- bool l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) &&
- (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL));
-
- ip_summed = CHECKSUM_UNNECESSARY;
- hash_type = PKT_HASH_TYPE_L4;
- if (l2_tunnel)
- skb->csum_level = 1;
- ring->csum_ok++;
- } else {
+ bool l2_tunnel;
+
+ if (!((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) &&
+ cqe->checksum == cpu_to_be16(0xffff)))
goto csum_none;
- }
+
+ l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) &&
+ (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL));
+ ip_summed = CHECKSUM_UNNECESSARY;
+ hash_type = PKT_HASH_TYPE_L4;
+ if (l2_tunnel)
+ skb->csum_level = 1;
+ ring->csum_ok++;
} else {
- if (priv->flags & MLX4_EN_FLAG_RX_CSUM_NON_TCP_UDP &&
- (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV4 |
+ if (!(priv->flags & MLX4_EN_FLAG_RX_CSUM_NON_TCP_UDP &&
+ (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV4 |
#if IS_ENABLED(CONFIG_IPV6)
- MLX4_CQE_STATUS_IPV6))) {
+ MLX4_CQE_STATUS_IPV6))))
#else
- 0))) {
+ 0))))
#endif
- if (check_csum(cqe, skb, va, dev->features)) {
- goto csum_none;
- } else {
- ip_summed = CHECKSUM_COMPLETE;
- hash_type = PKT_HASH_TYPE_L3;
- ring->csum_complete++;
- }
- } else {
goto csum_none;
- }
+ if (check_csum(cqe, skb, va, dev->features))
+ goto csum_none;
+ ip_summed = CHECKSUM_COMPLETE;
+ hash_type = PKT_HASH_TYPE_L3;
+ ring->csum_complete++;
}
} else {
csum_none:
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/4] net/mlx4_en: RX csum, remove redundant branches and checks
From: Tariq Toukan @ 2017-12-28 14:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <1514471171-3894-1-git-send-email-tariqt@mellanox.com>
Do not check IPv6 bit in cqe status if CONFIG_IPV6 is not enabled.
Function check_csum() is reached only with IPv4 or IPv6 set (if enabled),
if IPv6 is not set (or is not enabled) it is redundant to test the
IPv4 bit.
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 85e28efcda33..680bd3fbaa60 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -617,6 +617,10 @@ static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff *skb,
return 0;
}
#endif
+
+/* We reach this function only after checking that any of
+ * the (IPv4 | IPv6) bits are set in cqe->status.
+ */
static int check_csum(struct mlx4_cqe *cqe, struct sk_buff *skb, void *va,
netdev_features_t dev_features)
{
@@ -632,13 +636,11 @@ static int check_csum(struct mlx4_cqe *cqe, struct sk_buff *skb, void *va,
hdr += sizeof(struct vlan_hdr);
}
- if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV4))
- return get_fixed_ipv4_csum(hw_checksum, skb, hdr);
#if IS_ENABLED(CONFIG_IPV6)
if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV6))
return get_fixed_ipv6_csum(hw_checksum, skb, hdr);
#endif
- return 0;
+ return get_fixed_ipv4_csum(hw_checksum, skb, hdr);
}
int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int budget)
@@ -830,7 +832,11 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
} else {
if (priv->flags & MLX4_EN_FLAG_RX_CSUM_NON_TCP_UDP &&
(cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPV4 |
+#if IS_ENABLED(CONFIG_IPV6)
MLX4_CQE_STATUS_IPV6))) {
+#else
+ 0))) {
+#endif
if (check_csum(cqe, skb, va, dev->features)) {
goto csum_none;
} else {
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/4] net/mlx4_core: Cleanup FMR unmapping flow
From: Tariq Toukan @ 2017-12-28 14:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan, Moshe Shemesh
In-Reply-To: <1514471171-3894-1-git-send-email-tariqt@mellanox.com>
Remove redundant and not essential operations in fmr unmap/free.
According to device spec, in FMR unmap it is sufficient to set
ownership bit to SW. This allows remapping afterwards.
Fixes: 8ad11fb6b073 ("IB/mlx4: Implement FMRs")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/mr.c | 40 +++++++++++++++++----------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index c7c0764991c9..2e84f10f59ba 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -1103,30 +1103,16 @@ int mlx4_fmr_enable(struct mlx4_dev *dev, struct mlx4_fmr *fmr)
void mlx4_fmr_unmap(struct mlx4_dev *dev, struct mlx4_fmr *fmr,
u32 *lkey, u32 *rkey)
{
- struct mlx4_cmd_mailbox *mailbox;
- int err;
-
if (!fmr->maps)
return;
- fmr->maps = 0;
+ /* To unmap: it is sufficient to take back ownership from HW */
+ *(u8 *)fmr->mpt = MLX4_MPT_STATUS_SW;
- mailbox = mlx4_alloc_cmd_mailbox(dev);
- if (IS_ERR(mailbox)) {
- err = PTR_ERR(mailbox);
- pr_warn("mlx4_ib: mlx4_alloc_cmd_mailbox failed (%d)\n", err);
- return;
- }
+ /* Make sure MPT status is visible */
+ wmb();
- err = mlx4_HW2SW_MPT(dev, NULL,
- key_to_hw_index(fmr->mr.key) &
- (dev->caps.num_mpts - 1));
- mlx4_free_cmd_mailbox(dev, mailbox);
- if (err) {
- pr_warn("mlx4_ib: mlx4_HW2SW_MPT failed (%d)\n", err);
- return;
- }
- fmr->mr.enabled = MLX4_MPT_EN_SW;
+ fmr->maps = 0;
}
EXPORT_SYMBOL_GPL(mlx4_fmr_unmap);
@@ -1136,6 +1122,22 @@ int mlx4_fmr_free(struct mlx4_dev *dev, struct mlx4_fmr *fmr)
if (fmr->maps)
return -EBUSY;
+ if (fmr->mr.enabled == MLX4_MPT_EN_HW) {
+ /* In case of FMR was enabled and unmapped
+ * make sure to give ownership of MPT back to HW
+ * so HW2SW_MPT command will success.
+ */
+ *(u8 *)fmr->mpt = MLX4_MPT_STATUS_SW;
+ /* Make sure MPT status is visible before changing MPT fields */
+ wmb();
+ fmr->mpt->length = 0;
+ fmr->mpt->start = 0;
+ /* Make sure MPT data is visible after changing MPT status */
+ wmb();
+ *(u8 *)fmr->mpt = MLX4_MPT_STATUS_HW;
+ /* make sure MPT status is visible */
+ wmb();
+ }
ret = mlx4_mr_free(dev, &fmr->mr);
if (ret)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/4] net/mlx4_en: Change default QoS settings
From: Tariq Toukan @ 2017-12-28 14:26 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Eran Ben Elisha, Moni Shoua, Maor Gottlieb, Tariq Toukan
In-Reply-To: <1514471171-3894-1-git-send-email-tariqt@mellanox.com>
From: Moni Shoua <monis@mellanox.com>
Change the default mapping between TC and TCG as follows:
Prio | TC/TCG
| from to
| (set by FW) (set by SW)
---------+-----------------------------------
0 | 0/0 0/7
1 | 1/0 0/6
2 | 2/0 0/5
3 | 3/0 0/4
4 | 4/0 0/3
5 | 5/0 0/2
6 | 6/0 0/1
7 | 7/0 0/0
These new settings cause that a pause frame for any prio stops
traffic for all prios.
Fixes: 564c274c3df0 ("net/mlx4_en: DCB QoS support")
Signed-off-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c | 5 +++++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 7 +++++++
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
index 5f41dc92aa68..1a0c3bf86ead 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
@@ -310,6 +310,7 @@ static int mlx4_en_ets_validate(struct mlx4_en_priv *priv, struct ieee_ets *ets)
}
switch (ets->tc_tsa[i]) {
+ case IEEE_8021QAZ_TSA_VENDOR:
case IEEE_8021QAZ_TSA_STRICT:
break;
case IEEE_8021QAZ_TSA_ETS:
@@ -347,6 +348,10 @@ static int mlx4_en_config_port_scheduler(struct mlx4_en_priv *priv,
/* higher TC means higher priority => lower pg */
for (i = IEEE_8021QAZ_MAX_TCS - 1; i >= 0; i--) {
switch (ets->tc_tsa[i]) {
+ case IEEE_8021QAZ_TSA_VENDOR:
+ pg[i] = MLX4_EN_TC_VENDOR;
+ tc_tx_bw[i] = MLX4_EN_BW_MAX;
+ break;
case IEEE_8021QAZ_TSA_STRICT:
pg[i] = num_strict++;
tc_tx_bw[i] = MLX4_EN_BW_MAX;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 99051a294fa6..21bc17fa3854 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3336,6 +3336,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
priv->msg_enable = MLX4_EN_MSG_LEVEL;
#ifdef CONFIG_MLX4_EN_DCB
if (!mlx4_is_slave(priv->mdev->dev)) {
+ u8 prio;
+
+ for (prio = 0; prio < IEEE_8021QAZ_MAX_TCS; ++prio) {
+ priv->ets.prio_tc[prio] = prio;
+ priv->ets.tc_tsa[prio] = IEEE_8021QAZ_TSA_VENDOR;
+ }
+
priv->dcbx_cap = DCB_CAP_DCBX_VER_CEE | DCB_CAP_DCBX_HOST |
DCB_CAP_DCBX_VER_IEEE;
priv->flags |= MLX4_EN_DCB_ENABLED;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 2b72677eccd4..7db3d0d9bfce 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -479,6 +479,7 @@ struct mlx4_en_frag_info {
#define MLX4_EN_BW_MIN 1
#define MLX4_EN_BW_MAX 100 /* Utilize 100% of the line */
+#define MLX4_EN_TC_VENDOR 0
#define MLX4_EN_TC_ETS 7
enum dcb_pfc_type {
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface
From: Florian Fainelli @ 2017-12-28 15:02 UTC (permalink / raw)
To: Antoine Tenart, Andrew Lunn
Cc: thomas.petazzoni, ymarkman, jason, netdev, linux-kernel,
Russell King - ARM Linux, kishon, nadavh, miquel.raynal,
gregory.clement, stefanc, mw, davem, linux-arm-kernel,
sebastian.hesselbarth
In-Reply-To: <20171228100519.GE2626@kwain>
On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> Hi Andrew,
>
> On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
>>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
>>>>
>>>> +&cps_eth2 {
>>>> + /* CPS Lane 5 */
>>>> + status = "okay";
>>>> + phy-mode = "2500base-x";
>>>> + /* Generic PHY, providing serdes lanes */
>>>> + phys = <&cps_comphy5 2>;
>>>> +};
>>>> +
>>>
>>> This is wrong. This lane is connected to a SFP cage which can support
>>> more than just 2500base-X. Tying it in this way to 2500base-X means
>>> that this port does not support conenctions at 1000base-X, despite
>>> that's one of the most popular and more standardised speeds.
>>>
>>
>> I agree with Russell here. SFP modules are hot pluggable, and support
>> a range of interface modes. You need to query what the SFP module is
>> in order to know how to configure the SERDES interface. The phylink
>> infrastructure does that for you.
>
> Sure, I understand. We'll be able to support such interfaces only when
> the phylink PPv2 support lands in.
Should we expect PHYLINK support to make it as the first patch in your
v2 of this patch series, or is someone else doing that?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: Guillaume Nault @ 2017-12-28 14:53 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: davem, netdev, jchapman, liuhangbin
In-Reply-To: <d51dc65a40c5e335984e95a1e6db0eea00d9fd13.1513951129.git.lorenzo.bianconi@redhat.com>
On Fri, Dec 22, 2017 at 03:10:18PM +0100, Lorenzo Bianconi wrote:
> Introduce peer_offset parameter in order to add the capability
> to specify two different values for payload offset on tx/rx side.
> If just offset is provided by userspace use it for rx side as well
> in order to maintain compatibility with older l2tp versions
>
Sorry for being late on this, I originally missed this patchset and
only noticed it yesterday.
Lorenzo, can you give some context around this new feature?
Quite frankly I can't see the point of it. I've never heard of offsets
in L2TPv3, and for L2TPv2, the offset value is already encoded in the
header.
After a quick review of L2TPv3 and pseudowires RFCs, I still don't see
how adding some padding between the L2TPv3 header and the payload could
constitute a valid frame. Of course, the base feature is already there,
but after a quick test, it looks like the padding bits aren't
initialised and leak memory.
So, unless I missed something, setting offsets in L2TPv3 is
non-compliant, the current implementation is buggy and most likely
unused. I'd really prefer getting the implementation fixed, or even
removed entirely. Extending it to allow asymmetrical offset values
looks wrong to me, unless you have a use case in mind.
Regards,
Guillaume
PS: I also noticed that iproute2 has a "peer_offset" option, but it's a
noop.
^ permalink raw reply
* Re: [PATCH iproute2] gre/tunnel: Print erspan_index using print_uint()
From: William Tu @ 2017-12-28 15:13 UTC (permalink / raw)
To: Serhey Popovych; +Cc: Linux Kernel Network Developers
In-Reply-To: <1514459579-20379-1-git-send-email-serhe.popovych@gmail.com>
Hi Serhey,
On Thu, Dec 28, 2017 at 3:12 AM, Serhey Popovych
<serhe.popovych@gmail.com> wrote:
> One is missing in JSON output because fprintf()
> is used instead of print_uint().
>
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
> ip/link_gre.c | 3 ++-
> ip/link_gre6.c | 4 +++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ip/link_gre.c b/ip/link_gre.c
> index 896bb19..1e331c8 100644
> --- a/ip/link_gre.c
> +++ b/ip/link_gre.c
> @@ -476,7 +476,8 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
> if (tb[IFLA_GRE_ERSPAN_INDEX]) {
> __u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
>
> - fprintf(f, "erspan_index %u ", erspan_idx);
> + print_uint(PRINT_ANY,
> + "erspan_index", "erspan_index %u ", erspan_idx);
> }
>
Did you pull the latest code from net-next branch?
I think I already fix it.
William
^ permalink raw reply
* Re: [PATCH iproute2] gre/tunnel: Print erspan_index using print_uint()
From: Serhey Popovych @ 2017-12-28 15:48 UTC (permalink / raw)
To: William Tu; +Cc: Linux Kernel Network Developers
In-Reply-To: <CALDO+SYhqzg=rewp0qRBOLdz8r7QdN_aORHpz1o02PMiTcWiUQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1450 bytes --]
> Hi Serhey,
Hi William,
Yes, iproute2-next/net-next branch contains fix already: we probably do
not need proposed change. Sorry for noise.
I'm currently focused on bug fixing in stable which affects my work for
iproute2-next. All my future work will be based on iproute2-next as it
contains refactoring, optimization and cleanups, not fixes.
Sorry again and thanks for feedback.
>
> On Thu, Dec 28, 2017 at 3:12 AM, Serhey Popovych
> <serhe.popovych@gmail.com> wrote:
>> One is missing in JSON output because fprintf()
>> is used instead of print_uint().
>>
>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>> ---
>> ip/link_gre.c | 3 ++-
>> ip/link_gre6.c | 4 +++-
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/ip/link_gre.c b/ip/link_gre.c
>> index 896bb19..1e331c8 100644
>> --- a/ip/link_gre.c
>> +++ b/ip/link_gre.c
>> @@ -476,7 +476,8 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>> if (tb[IFLA_GRE_ERSPAN_INDEX]) {
>> __u32 erspan_idx = rta_getattr_u32(tb[IFLA_GRE_ERSPAN_INDEX]);
>>
>> - fprintf(f, "erspan_index %u ", erspan_idx);
>> + print_uint(PRINT_ANY,
>> + "erspan_index", "erspan_index %u ", erspan_idx);
>> }
>>
>
> Did you pull the latest code from net-next branch?
> I think I already fix it.
> William
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* [patch net-next] net: sched: don't set extack message in case the qdisc will be created
From: Jiri Pirko @ 2017-12-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, mlxsw, aring
From: Jiri Pirko <jiri@mellanox.com>
If the qdisc is not found here, it is going to be created. Therefore,
this is not an error path. Remove the extack message set and don't
confuse user with error message in case the qdisc was created
successfully.
Fixes: 09215598119e ("net: sched: sch_api: handle generic qdisc errors")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/sch_api.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 3a3a1da..81ecf5b 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1402,10 +1402,8 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
return -EINVAL;
}
q = qdisc_lookup(dev, tcm->tcm_handle);
- if (!q) {
- NL_SET_ERR_MSG(extack, "No qdisc found for specified handle");
+ if (!q)
goto create_n_graft;
- }
if (n->nlmsg_flags & NLM_F_EXCL) {
NL_SET_ERR_MSG(extack, "Exclusivity flag on, cannot override");
return -EEXIST;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net-next v2 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: Jason Baron @ 2017-12-28 15:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, virtualization, qemu-devel, mst, jasowang
In-Reply-To: <20171227.164334.2079471466039224265.davem@davemloft.net>
On 12/27/2017 04:43 PM, David Miller wrote:
> From: Jason Baron <jbaron@akamai.com>
> Date: Fri, 22 Dec 2017 16:54:01 -0500
>
>> The ability to set speed and duplex for virtio_net in useful in various
>> scenarios as described here:
>>
>> 16032be virtio_net: add ethtool support for set and get of settings
>>
>> However, it would be nice to be able to set this from the hypervisor,
>> such that virtio_net doesn't require custom guest ethtool commands.
>>
>> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
>> the hypervisor to export a linkspeed and duplex setting. The user can
>> subsequently overwrite it later if desired via: 'ethtool -s'.
>>
>> Signed-off-by: Jason Baron <jbaron@akamai.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>
> Looks mostly fine to me but need some virtio_net reviewers on this one.
>
>> @@ -57,6 +57,8 @@
>> * Steering */
>> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>>
>> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Host set linkspeed and duplex */
>> +
>
> Why use a value so far away from the largest existing one?
>
> Just curious.
>
So that came from a discussion with Michael about which bit to use for
this, and he suggested using 63:
"
Transports started from bit 24 and are growing up.
So I would say devices should start from bit 63 and grow down.
"
https://patchwork.ozlabs.org/patch/848814/#1826669
I will add a comment to explain it.
Thanks,
-Jason
^ 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