* Re: [PATCH v2 net-next] veth: Free queues on link delete
From: Toshiaki Makita @ 2018-08-15 1:16 UTC (permalink / raw)
To: dsahern, netdev; +Cc: davem, David Ahern
In-Reply-To: <20180815010418.5521-1-dsahern@kernel.org>
On 2018/08/15 10:04, dsahern@kernel.org wrote:
> From: David Ahern <dsahern@gmail.com>
>
> kmemleak reported new suspected memory leaks.
> $ cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff8800354d5c00 (size 1024):
> comm "ip", pid 836, jiffies 4294722952 (age 25.904s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<(____ptrval____)>] kmemleak_alloc+0x70/0x94
> [<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
> [<(____ptrval____)>] __kmalloc+0x101/0x142
> [<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
> [<(____ptrval____)>] veth_newlink+0x147/0x3ac [veth]
> ...
> unreferenced object 0xffff88002e009c00 (size 1024):
> comm "ip", pid 836, jiffies 4294722958 (age 25.898s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<(____ptrval____)>] kmemleak_alloc+0x70/0x94
> [<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
> [<(____ptrval____)>] __kmalloc+0x101/0x142
> [<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
> [<(____ptrval____)>] veth_newlink+0x219/0x3ac [veth]
>
> The allocations in question are veth_alloc_queues for the dev and its peer.
>
> Free the queues on a delete.
>
> Fixes: 638264dc90227 ("veth: Support per queue XDP ring")
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> v2
> - free peer dev queues as well
>
> drivers/net/veth.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index e3202af72df5..2a3ce60631ef 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1205,6 +1205,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
> struct veth_priv *priv;
> struct net_device *peer;
>
> + veth_free_queues(dev);
> priv = netdev_priv(dev);
> peer = rtnl_dereference(priv->peer);
>
> @@ -1216,6 +1217,7 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
> unregister_netdevice_queue(dev, head);
>
> if (peer) {
> + veth_free_queues(peer);
> priv = netdev_priv(peer);
> RCU_INIT_POINTER(priv->peer, NULL);
> unregister_netdevice_queue(peer, head);
Hmm, on second thought this queues need to be freed after veth_close()
to make sure no packet will reference them. That means we need to free
them in .ndo_uninit() or destructor.
(rtnl_delete_link() calls dellink() before unregister_netdevice_many()
which calls dev_close_many() through rollback_registered_many())
Currently veth has destructor veth_dev_free() for vstats, so we can free
queues in the function.
To be in line with vstats, allocation also should be moved to
veth_dev_init().
--
Toshiaki Makita
^ permalink raw reply
* Re: samples don't build on v4.18
From: Joel Fernandes @ 2018-08-15 1:22 UTC (permalink / raw)
To: LKML, wangnan0, open list:BPF (Safe dynamic programs and tools),
Alexei Starovoitov, acme, Chenbo Feng, Jakub Kicinski
In-Reply-To: <20180815012054.GA70201@joelaf.mtv.corp.google.com>
Forgot to add the patch author, doing so now. thanks
On Tue, Aug 14, 2018 at 6:20 PM, Joel Fernandes <joelaf@google.com> wrote:
>
> Hi,
> When building BPF samples on v4.18, I get the following errors:
>
> $ cd samples/bpf/
> $ make
>
> Auto-detecting system features:
> ... libelf: [ OFF ]
> ... bpf: [ OFF ]
>
> No libelf found
> Makefile:213: recipe for target 'elfdep' failed
> -----------
>
> I bissected it down to commit 5f9380572b4bb24f60cd492b1
>
> Author: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Thu May 10 10:24:39 2018 -0700
>
> samples: bpf: compile and link against full libbpf
> ---------
>
> Checking out a kernel before this commit makes the samples build. Also I do
> have libelf on my system.
>
> Any thoughts on this issue?
>
> thank you,
>
> - Joel
^ permalink raw reply
* Re: [PATCH v2 net-next] veth: Free queues on link delete
From: David Ahern @ 2018-08-15 1:29 UTC (permalink / raw)
To: Toshiaki Makita, dsahern, netdev; +Cc: davem
In-Reply-To: <ed81401b-e049-d1ae-d635-1adcc8bd3b15@lab.ntt.co.jp>
On 8/14/18 7:16 PM, Toshiaki Makita wrote:
> Hmm, on second thought this queues need to be freed after veth_close()
> to make sure no packet will reference them. That means we need to free
> them in .ndo_uninit() or destructor.
> (rtnl_delete_link() calls dellink() before unregister_netdevice_many()
> which calls dev_close_many() through rollback_registered_many())
>
> Currently veth has destructor veth_dev_free() for vstats, so we can free
> queues in the function.
> To be in line with vstats, allocation also should be moved to
> veth_dev_init().
given that, can you take care of the free in the proper location?
^ permalink raw reply
* Re: [PATCH v2 net-next] veth: Free queues on link delete
From: Toshiaki Makita @ 2018-08-15 1:32 UTC (permalink / raw)
To: David Ahern, dsahern, netdev; +Cc: davem
In-Reply-To: <990e3b33-02b9-c6a6-a48e-8fcc52bdcde9@gmail.com>
On 2018/08/15 10:29, David Ahern wrote:
> On 8/14/18 7:16 PM, Toshiaki Makita wrote:
>> Hmm, on second thought this queues need to be freed after veth_close()
>> to make sure no packet will reference them. That means we need to free
>> them in .ndo_uninit() or destructor.
>> (rtnl_delete_link() calls dellink() before unregister_netdevice_many()
>> which calls dev_close_many() through rollback_registered_many())
>>
>> Currently veth has destructor veth_dev_free() for vstats, so we can free
>> queues in the function.
>> To be in line with vstats, allocation also should be moved to
>> veth_dev_init().
>
> given that, can you take care of the free in the proper location?
Sure, will cook a patch.
Thanks!
--
Toshiaki Makita
^ permalink raw reply
* Re: [PATCH RFC/RFT net-next 00/17] net: Convert neighbor tables to per-namespace
From: Eric W. Biederman @ 2018-08-15 4:36 UTC (permalink / raw)
To: David Ahern
Cc: Cong Wang, David Miller, Linux Kernel Network Developers,
nikita.leshchenko, Roopa Prabhu, Stephen Hemminger, Ido Schimmel,
Jiri Pirko, Saeed Mahameed, Alexander Aring, linux-wpan,
NetFilter, LKML
In-Reply-To: <a227d23c-40a1-97a9-d39c-70d16cd470b4@gmail.com>
David Ahern <dsahern@gmail.com> writes:
> On 7/25/18 1:17 PM, Eric W. Biederman wrote:
>> David Ahern <dsahern@gmail.com> writes:
>>
>>> On 7/25/18 11:38 AM, Eric W. Biederman wrote:
>>>>
>>>> Absolutely NOT. Global thresholds are exactly correct given the fact
>>>> you are running on a single kernel.
>>>>
>>>> Memory is not free (Even though we are swimming in enough of it memory
>>>> rarely matters). One of the few remaining challenges is for containers
>>>> is finding was to limit resources in such a way that one application
>>>> does not mess things up for another container during ordinary usage.
>>>>
>>>> It looks like the neighbour tables absolutely are that kind of problem,
>>>> because the artificial limits are too strict. Completely giving up on
>>>> limits does not seem right approach either. We need to fix the limits
>>>> we have (perhaps making them go away entirely), not just apply a
>>>> band-aid. Let's get to the bottom of this and make the system better.
>>>
>>> Eric: yes, they all share the global resource of memory and there should
>>> be limits on how many entries a remote entity can create.
>>>
>>> Network namespaces can provide a separation such that one namespace does
>>> not disrupt networking in another. It is absolutely appropriate to do
>>> so. Your rigid stance is inconsistent given the basic meaning of a
>>> network namespace and the parallels to this same problem -- bridges,
>>> vxlans, and ip fragments. Only neighbor tables are not per-device or per
>>> namespace; your insistence on global limits is missing the mark and wrong.
>>
>> That is not what I said. Let me rephrase and see if you understand.
>>
>> The problem appears to be of lots of devices. Fundamentally if you use
>> lots of network devices today unless you adjust gc_thresh3 you will run
>> out of neighbour table entries.
>>
>> The problem has a bigger scope than what you are looking at.
>>
>> If you fix the core problem you won't see the problem in the context
>> of network namespaces either.
>>
>> Default limits should be something that will never be hit unless
>> something goes crazy. We are hitting them. Therefore by definition
>> there is a bug in these limits.
>
> I disagree that the problem is a global limit. It is trivial for users
> to increase gc_thresh3. That does not solve the fundamental problem.
>
>>
>>
>> And yes there is absolutely a place for global limits on things like
>> inodes, file descriptors etc, that does not care about which part of the
>> kernel you are in. However hitting those limits in normal operation is
>> a bug.
>>
>> We have ourselves a bug.
>
> I agree we have a bug; we disagree on what that bug is.
>
> I am just back from vacation and re-read your responses. No where do you
> acknowledge the fundamental point of this patch set - that adding a new
> neighbor entry in one namespace can evict an entry in another namespace
> or worse networking in one namespace can fail due to table overflow
> because of entries from another. That is a real problem.
>
> It is not a matter of increasing the default gc_thresh3 to some number
> N; it is ensuring that regardless of the value of gc_thresh3 one
> namespace is not affected by another.
My suggestion is to look at the problem and it's requirements and figure
out how to safely remove gc_thresh3 entirely. We do have to ensure
neighbour tables don't grow too large, I expect we can do it in a way
that can scale from a small machine with few neighbours to a large
machine with many neighbours.
Perhaps the code just needs to limit the number of neighbours who have
never replied and the code is probing for an a per interface basis.
It still may make sense to have a global limit of perhaps a million
entries just because that would be an indicator that something has truly
gone weird.
> You created network namespaces and it provides isolation -- separate
> tables essentially -- for devices, FIB entries, sockets, etc, but you
> argue against completing the task with separate neighbor tables which is
> very strange given the impact (completely broken networking).
Namespaces provide isolation at the level of names. The objects still
share a kernel and compete for resources. Not competing for resources
would require each namespace have it's own dedicated pool of resources
which over the whole machine would be much less efficient.
That is the fundamental design difference between namespaces and VM's
and it is why namespaces can be much cheaper and much more resource
efficient. Reserving your worst case resource usage ahead of time tends
to result in a lot of inefficiencies.
Eric
^ permalink raw reply
* [PATCH 1/1] vhost: change the signature of __vhost_get_user_slow()
From: Dongli Zhang @ 2018-08-15 1:46 UTC (permalink / raw)
To: kvm, netdev; +Cc: linux-kernel, mst, jasowang, virtualization
Remove 'type' from the signature of __vhost_get_user_slow() as it is not
used.
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
drivers/vhost/vhost.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ed31145..f78d3bc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -807,8 +807,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
}
static void __user *__vhost_get_user_slow(struct vhost_virtqueue *vq,
- void __user *addr, unsigned int size,
- int type)
+ void __user *addr, unsigned int size)
{
int ret;
@@ -846,7 +845,7 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
if (uaddr)
return uaddr;
- return __vhost_get_user_slow(vq, addr, size, type);
+ return __vhost_get_user_slow(vq, addr, size);
}
#define vhost_put_user(vq, x, ptr) \
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Brad Mouring @ 2018-08-15 2:03 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Ahmad Fatoum, David S. Miller, Nicolas Ferre, netdev, mdf, stable,
kernel, Andrew Lunn, Florian Fainelli
In-Reply-To: <20180814155812.cjuacvkhqeuctcry@pengutronix.de>
Hello Ahmed, Uwe,
On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
> Hello Ahmad,
>
>
> On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
> > The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
> > There, of_mdiobus_register was called even for the fixed-link representing
> > the SPI-connected switch PHY, with the result that the driver attempts to
> > enumerate PHYs on a non-existent MDIO bus:
> >
> > libphy: MACB_mii_bus: probed
> > mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> > mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
> > [snip]
> > mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
> > macb f0028000.ethernet: broken fixed-link specification
> >
> > Cc: <stable@vger.kernel.org>
> > Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
>
> I added the people involved in 739de9a1563a to Cc. Maybe they want to
> comment. So not stripping the remaining part of the original mail.
>
> Best regards
> Uwe
You should probably prod Andrew Lunn, he suggested that I move the fixed
link code from macb_mii_init() to _probe(). Here, you're at least partially
directly undoing that.
(ref: https://www.mail-archive.com/netdev@vger.kernel.org/msg221018.html)
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> > drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++++---------
> > 1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index a6c911bb5ce2..d202a03c42ed 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -481,11 +481,6 @@ static int macb_mii_probe(struct net_device *dev)
> >
> > if (np) {
> > if (of_phy_is_fixed_link(np)) {
> > - if (of_phy_register_fixed_link(np) < 0) {
> > - dev_err(&bp->pdev->dev,
> > - "broken fixed-link specification\n");
> > - return -ENODEV;
> > - }
> > bp->phy_node = of_node_get(np);
> > } else {
> > bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
> > @@ -568,7 +563,7 @@ static int macb_mii_init(struct macb *bp)
> > {
> > struct macb_platform_data *pdata;
> > struct device_node *np;
> > - int err;
> > + int err = -ENXIO;
> >
> > /* Enable management port */
> > macb_writel(bp, NCR, MACB_BIT(MPE));
> > @@ -591,10 +586,21 @@ static int macb_mii_init(struct macb *bp)
> > dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
> >
> > np = bp->pdev->dev.of_node;
> > - if (pdata)
> > - bp->mii_bus->phy_mask = pdata->phy_mask;
> > + if (np && of_phy_is_fixed_link(np)) {
> > + if (of_phy_register_fixed_link(np) < 0) {
> > + dev_err(&bp->pdev->dev,
> > + "broken fixed-link specification\n");
> > + goto err_out_free_mdiobus;
> > + }
> > +
> > + err = mdiobus_register(bp->mii_bus);
> > + } else {
> > + if (pdata)
> > + bp->mii_bus->phy_mask = pdata->phy_mask;
> > +
> > + err = of_mdiobus_register(bp->mii_bus, np);
> > + }
> >
> > - err = of_mdiobus_register(bp->mii_bus, np);
> > if (err)
> > goto err_out_free_mdiobus;
> >
> > @@ -606,9 +612,9 @@ static int macb_mii_init(struct macb *bp)
> >
> > err_out_unregister_bus:
> > mdiobus_unregister(bp->mii_bus);
> > +err_out_free_mdiobus:
> > if (np && of_phy_is_fixed_link(np))
> > of_phy_deregister_fixed_link(np);
> > -err_out_free_mdiobus:
> > of_node_put(bp->phy_node);
> > mdiobus_free(bp->mii_bus);
> > err_out:
> > --
> > 2.18.0
> >
> >
> >
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://urldefense.proofpoint.com/v2/url?u=http-3A__www.pengutronix.de_&d=DwIDAw&c=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=8iZb7YNOSMVIG_mTIHDL03ZcObgQI_gGlWrSewdGETA&m=IQAK1YsKs7Z2bvZxuajiSXw3asFiEztQKYkvy-LpBn8&s=XKrOhFxQshoEcDxMZVSATnJW2cbaD16mQofKdEbJVW0&e= |
--
Brad Mouring
Senior Software Engineer
National Instruments
512-683-6610 / bmouring@ni.com
^ permalink raw reply
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Andrew Lunn @ 2018-08-15 2:32 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Ahmad Fatoum, David S. Miller, Nicolas Ferre, netdev, mdf, stable,
kernel, Brad Mouring, Florian Fainelli
In-Reply-To: <20180814155812.cjuacvkhqeuctcry@pengutronix.de>
On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
> Hello Ahmad,
>
>
> On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
> > The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
> > There, of_mdiobus_register was called even for the fixed-link representing
> > the SPI-connected switch PHY, with the result that the driver attempts to
> > enumerate PHYs on a non-existent MDIO bus:
> >
> > libphy: MACB_mii_bus: probed
> > mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> > mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
> > [snip]
> > mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
> > macb f0028000.ethernet: broken fixed-link specification
> >
> > Cc: <stable@vger.kernel.org>
> > Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
>
> I added the people involved in 739de9a1563a to Cc. Maybe they want to
> comment. So not stripping the remaining part of the original mail.
Thanks Uwe for Cc: in my.
Ahmed, where is the device tree for the EVB-KSZ9477?
Thanks
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next V2 0/6] XDP rx handler
From: Alexei Starovoitov @ 2018-08-15 5:35 UTC (permalink / raw)
To: Jason Wang
Cc: David Ahern, Jesper Dangaard Brouer, netdev, linux-kernel, ast,
daniel, mst
In-Reply-To: <aa9cf883-7822-70a7-5ab5-c873b69c2098@redhat.com>
On Wed, Aug 15, 2018 at 08:29:45AM +0800, Jason Wang wrote:
>
> Looks less flexible since the topology is hard coded in the XDP program
> itself and this requires all logic to be implemented in the program on the
> root netdev.
>
> >
> > I have L3 forwarding working for vlan devices and bonds. I had not
> > considered macvlans specifically yet, but it should be straightforward
> > to add.
> >
>
> Yes, and all these could be done through XDP rx handler as well, and it can
> do even more with rather simple logic:
>
> 1 macvlan has its own namespace, and want its own bpf logic.
> 2 Ruse the exist topology information for dealing with more complex setup
> like macvlan on top of bond and team. There's no need to bpf program to care
> about topology. If you look at the code, there's even no need to attach XDP
> on each stacked device. The calling of xdp_do_pass() can try to pass XDP
> buff to upper device even if there's no XDP program attached to current
> layer.
> 3 Deliver XDP buff to userspace through macvtap.
I think I'm getting what you're trying to achieve.
You actually don't want any bpf programs in there at all.
You want macvlan builtin logic to act on raw packet frames.
It would have been less confusing if you said so from the beginning.
I think there is little value in such work, since something still
needs to process this raw frames eventually. If it's XDP with BPF progs
than they can maintain the speed, but in such case there is no need
for macvlan. The first layer can be normal xdp+bpf+xdp_redirect just fine.
In case where there is no xdp+bpf in final processing, the frames are
converted to skb and performance is lost, so in such cases there is no
need for builtin macvlan acting on raw xdp frames either. Just keep
existing macvlan acting on skbs.
^ permalink raw reply
* Re: samples don't build on v4.18
From: Joel Fernandes @ 2018-08-15 3:01 UTC (permalink / raw)
To: LKML, wangnan0, open list:BPF (Safe dynamic programs and tools),
Alexei Starovoitov, acme, Chenbo Feng, Jakub Kicinski
In-Reply-To: <CAJWu+orb35w6O5B9KyHdYtwc2RbaR9d72HW-i5Bsup72QP8Oag@mail.gmail.com>
On Tue, Aug 14, 2018 at 06:22:21PM -0700, Joel Fernandes wrote:
> Forgot to add the patch author, doing so now. thanks
>
> On Tue, Aug 14, 2018 at 6:20 PM, Joel Fernandes <joelaf@google.com> wrote:
> >
> > Hi,
> > When building BPF samples on v4.18, I get the following errors:
> >
> > $ cd samples/bpf/
> > $ make
> >
> > Auto-detecting system features:
> > ... libelf: [ OFF ]
> > ... bpf: [ OFF ]
> >
> > No libelf found
> > Makefile:213: recipe for target 'elfdep' failed
> > -----------
> >
> > I bissected it down to commit 5f9380572b4bb24f60cd492b1
> >
> > Author: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Date: Thu May 10 10:24:39 2018 -0700
> >
> > samples: bpf: compile and link against full libbpf
> > ---------
> >
> > Checking out a kernel before this commit makes the samples build. Also I do
> > have libelf on my system.
> >
> > Any thoughts on this issue?
There is some weirdness going on with my kernel tree. If I do a fresh clone
of v4.18 and build samples, everything works.
However if I take my existing checkout, do a:
git clean -f -d
make mrproper
Then I try to build the samples, I get the "No libelf found".
Obviously the existing checked out kernel tree is in some weird state that I
am not yet able to fix. But atleast if I blow the whole tree and clone again,
I'm able to build...
Is this related to the intermittent "No libelf found" issues that were
recently discussed?
- Joel
^ permalink raw reply
* Re: linux-next: manual merge of the devicetree tree with the net-next tree
From: Stephen Rothwell @ 2018-08-15 3:14 UTC (permalink / raw)
To: David Miller, Networking
Cc: Rob Herring, Linux-Next Mailing List, Linux Kernel Mailing List,
Anssi Hannula
In-Reply-To: <20180730143645.7bcd563b@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]
Hi all,
On Mon, 30 Jul 2018 14:37:22 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the devicetree tree got a conflict in:
>
> Documentation/devicetree/bindings/net/can/xilinx_can.txt
>
> between commit:
>
> 7cb0f17f5252 ("dt-bindings: can: xilinx_can: add Xilinx CAN FD bindings")
>
> from the net-next tree and commit:
>
> 791d3ef2e111 ("dt-bindings: remove 'interrupt-parent' from bindings")
>
> from the devicetree tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc Documentation/devicetree/bindings/net/can/xilinx_can.txt
> index ae5c07e96ad5,9264d2f6a89d..000000000000
> --- a/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> @@@ -2,26 -2,19 +2,25 @@@ Xilinx Axi CAN/Zynq CANPS controller De
> ---------------------------------------------------------
>
> Required properties:
> -- compatible : Should be "xlnx,zynq-can-1.0" for Zynq CAN
> - controllers and "xlnx,axi-can-1.00.a" for Axi CAN
> - controllers.
> -- reg : Physical base address and size of the Axi CAN/Zynq
> - CANPS registers map.
> +- compatible : Should be:
> + - "xlnx,zynq-can-1.0" for Zynq CAN controllers
> + - "xlnx,axi-can-1.00.a" for Axi CAN controllers
> + - "xlnx,canfd-1.0" for CAN FD controllers
> +- reg : Physical base address and size of the controller
> + registers map.
> - interrupts : Property with a value describing the interrupt
> number.
> - - interrupt-parent : Must be core interrupt controller
> -- clock-names : List of input clock names - "can_clk", "pclk"
> - (For CANPS), "can_clk" , "s_axi_aclk"(For AXI CAN)
> +- clock-names : List of input clock names
> + - "can_clk", "pclk" (For CANPS),
> + - "can_clk", "s_axi_aclk" (For AXI CAN and CAN FD).
> (See clock bindings for details).
> - clocks : Clock phandles (see clock bindings for details).
> -- tx-fifo-depth : Can Tx fifo depth.
> -- rx-fifo-depth : Can Rx fifo depth.
> +- tx-fifo-depth : Can Tx fifo depth (Zynq, Axi CAN).
> +- rx-fifo-depth : Can Rx fifo depth (Zynq, Axi CAN, CAN FD in
> + sequential Rx mode).
> +- tx-mailbox-count : Can Tx mailbox buffer count (CAN FD).
> +- rx-mailbox-count : Can Rx mailbox buffer count (CAN FD in mailbox Rx
> + mode).
>
>
> Example:
This is now a conflict between Linus' tree and the net-next tree.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] r8169: don't use MSI-X on RTL8106e
From: jian-hong @ 2018-08-15 6:21 UTC (permalink / raw)
To: Realtek linux nic maintainers, David S. Miller, netdev,
linux-kernel, linux
Cc: Jian-Hong Pan
From: Jian-Hong Pan <jian-hong@endlessm.com>
Found the ethernet network on ASUS X441UAR doesn't come back on resume
from suspend when using MSI-X. The chip is RTL8106e - version 39.
asus@endless:~$ dmesg | grep r8169
[ 21.848357] libphy: r8169: probed
[ 21.848473] r8169 0000:02:00.0 eth0: RTL8106e, 0c:9d:92:32:67:b4, XID
44900000, IRQ 127
[ 22.518860] r8169 0000:02:00.0 enp2s0: renamed from eth0
[ 29.458041] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
[ 63.227398] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off
[ 124.514648] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
Here is the ethernet controller in detail:
asus@endless:~$ sudo lspci -nnvs 02:00.0
[sudo] password for asus:
02:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
RTL8101/2/6E PCI Express Fast/Gigabit Ethernet controller [10ec:8136]
(rev 07)
Subsystem: ASUSTeK Computer Inc. RTL810xE PCI Express Fast
Ethernet controller [1043:200f]
Flags: bus master, fast devsel, latency 0, IRQ 16
I/O ports at e000 [size=256]
Memory at ef100000 (64-bit, non-prefetchable) [size=4K]
Memory at e0000000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [70] Express Endpoint, MSI 01
Capabilities: [b0] MSI-X: Enable+ Count=4 Masked-
Capabilities: [d0] Vital Product Data
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Virtual Channel
Capabilities: [160] Device Serial Number 01-00-00-00-36-4c-e0-00
Capabilities: [170] Latency Tolerance Reporting
Kernel driver in use: r8169
Kernel modules: r8169
Here is the system interrupt table:
asus@endless:~$ cat /proc/interrupts
CPU0 CPU1 CPU2 CPU3
0: 22 0 0 0 IO-APIC 2-edge
timer
1: 157 42 0 0 IO-APIC 1-edge
i8042
8: 0 0 1 0 IO-APIC 8-edge
rtc0
9: 10 13 0 0 IO-APIC 9-fasteoi
acpi
16: 0 0 0 0 IO-APIC 16-fasteoi
i2c_designware.0, i801_smbus
17: 2445 0 3453 0 IO-APIC 17-fasteoi
i2c_designware.1, rtl_pci
109: 2 0 0 1 IO-APIC 109-fasteoi
FTE1200:00
120: 0 0 0 0 PCI-MSI 458752-edge
PCIe PME
121: 0 0 0 0 PCI-MSI 466944-edge
PCIe PME
122: 0 0 0 0 PCI-MSI 468992-edge
PCIe PME
123: 1465 0 0 21263 PCI-MSI 376832-edge
ahci[0000:00:17.0]
124: 0 530 0 0 PCI-MSI 327680-edge
xhci_hcd
125: 5204 0 0 0 PCI-MSI 32768-edge
i915
126: 0 0 149 0 PCI-MSI 514048-edge
snd_hda_intel:card0
127: 0 0 337 0 PCI-MSI 1048576-edge
enp2s0
NMI: 0 0 0 0 Non-maskable
interrupts
LOC: 45049 39474 38978 46677 Local timer
interrupts
SPU: 0 0 0 0 Spurious interrupts
PMI: 0 0 0 0 Performance
monitoring interrupts
IWI: 619 8 0 1 IRQ work interrupts
RTR: 6 0 0 0 APIC ICR read
retries
RES: 4918 4436 3835 2943 Rescheduling
interrupts
CAL: 1399 1478 1598 1465 Function call
interrupts
TLB: 608 513 723 559 TLB shootdowns
TRM: 0 0 0 0 Thermal event
interrupts
THR: 0 0 0 0 Threshold APIC
interrupts
DFR: 0 0 0 0 Deferred Error APIC
interrupts
MCE: 0 0 0 0 Machine check
exceptions
MCP: 3 4 4 4 Machine check polls
ERR: 0
MIS: 0
PIN: 0 0 0 0 Posted-interrupt
notification event
NPI: 0 0 0 0 Nested
posted-interrupt event
PIW: 0 0 0 0 Posted-interrupt
wakeup event
It is the IRQ 127 - PCI-MSI used by enp2s0. However, lspci lists MSI is
disabled and MSI-X is enabled which conflicts to the interrupt table.
Falling back to MSI fixes the issue.
Here is the test result with this patch in dmesg:
asus@endless:~$ dmesg | grep r8169
[ 22.017477] libphy: r8169: probed
[ 22.017735] r8169 0000:02:00.0 eth0: RTL8106e, 0c:9d:92:32:67:b4, XID
44900000, IRQ 127
[ 22.041489] r8169 0000:02:00.0 enp2s0: renamed from eth0
[ 29.138312] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
[ 30.927359] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off
[ 289.998077] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off
[ 290.508084] Generic PHY r8169-200:00: attached PHY driver [Generic
PHY] (mii_bus:phy_addr=r8169-200:00, irq=IGNORE)
[ 290.745690] r8169 0000:02:00.0 enp2s0: Link is Down
[ 292.367717] r8169 0000:02:00.0 enp2s0: Link is Up - 100Mbps/Full -
flow control off
lspci lists MSI is enabled and MSI-X is disabled with this patch:
asus@endless:~/linux-net$ sudo lspci -nnvs 02:00.0
[sudo] password for asus:
02:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
RTL8101/2/6E PCI Express Fast/Gigabit Ethernet controller [10ec:8136]
(rev 07)
Subsystem: ASUSTeK Computer Inc. RTL810xE PCI Express Fast
Ethernet controller [1043:200f]
Flags: bus master, fast devsel, latency 0, IRQ 127
I/O ports at e000 [size=256]
Memory at ef100000 (64-bit, non-prefetchable) [size=4K]
Memory at e0000000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [70] Express Endpoint, MSI 01
Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
Capabilities: [d0] Vital Product Data
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Virtual Channel
Capabilities: [160] Device Serial Number 01-00-00-00-36-4c-e0-00
Capabilities: [170] Latency Tolerance Reporting
Kernel driver in use: r8169
Kernel modules: r8169
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/net/ethernet/realtek/r8169.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 0d9c3831838f..0efa977c422d 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7071,17 +7071,20 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
{
unsigned int flags;
- if (tp->mac_version <= RTL_GIGA_MAC_VER_06) {
+ switch (tp->mac_version) {
+ case RTL_GIGA_MAC_VER_01 ... RTL_GIGA_MAC_VER_06:
RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~MSIEnable);
RTL_W8(tp, Cfg9346, Cfg9346_Lock);
flags = PCI_IRQ_LEGACY;
- } else if (tp->mac_version == RTL_GIGA_MAC_VER_40) {
+ break;
+ case RTL_GIGA_MAC_VER_39 ... RTL_GIGA_MAC_VER_40:
/* This version was reported to have issues with resume
* from suspend when using MSI-X
*/
flags = PCI_IRQ_LEGACY | PCI_IRQ_MSI;
- } else {
+ break;
+ default:
flags = PCI_IRQ_ALL_TYPES;
}
--
2.11.0
^ permalink raw reply related
* Re: [RFC PATCH net-next V2 0/6] XDP rx handler
From: Jason Wang @ 2018-08-15 7:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Ahern, Jesper Dangaard Brouer, netdev, linux-kernel, ast,
daniel, mst, Toshiaki Makita
In-Reply-To: <20180815053550.5g4f5qeb7r4wtgm5@ast-mbp>
On 2018年08月15日 13:35, Alexei Starovoitov wrote:
> On Wed, Aug 15, 2018 at 08:29:45AM +0800, Jason Wang wrote:
>> Looks less flexible since the topology is hard coded in the XDP program
>> itself and this requires all logic to be implemented in the program on the
>> root netdev.
>>
>>> I have L3 forwarding working for vlan devices and bonds. I had not
>>> considered macvlans specifically yet, but it should be straightforward
>>> to add.
>>>
>> Yes, and all these could be done through XDP rx handler as well, and it can
>> do even more with rather simple logic:
>>
>> 1 macvlan has its own namespace, and want its own bpf logic.
>> 2 Ruse the exist topology information for dealing with more complex setup
>> like macvlan on top of bond and team. There's no need to bpf program to care
>> about topology. If you look at the code, there's even no need to attach XDP
>> on each stacked device. The calling of xdp_do_pass() can try to pass XDP
>> buff to upper device even if there's no XDP program attached to current
>> layer.
>> 3 Deliver XDP buff to userspace through macvtap.
> I think I'm getting what you're trying to achieve.
> You actually don't want any bpf programs in there at all.
> You want macvlan builtin logic to act on raw packet frames.
The built-in logic is just used to find the destination macvlan device.
It could be done by through another bpf program. Instead of inventing
lots of generic infrastructure on kernel with specific userspace API,
built-in logic has its own advantages:
- support hundreds or even thousands of macvlans
- using exist tools to configure network
- immunity to topology changes
> It would have been less confusing if you said so from the beginning.
The name "XDP rx handler" is probably not good. Something like "stacked
deivce XDP" might be better.
> I think there is little value in such work, since something still
> needs to process this raw frames eventually. If it's XDP with BPF progs
> than they can maintain the speed, but in such case there is no need
> for macvlan. The first layer can be normal xdp+bpf+xdp_redirect just fine.
I'm a little bit confused. We allow per veth XDP program, so I believe
per macvlan XDP program makes sense as well? This allows great
flexibility and there's no need to care about topology in bpf program.
The configuration is also greatly simplified. The only difference is we
can use xdp_redirect for veth since it was pair device, we can transmit
XDP frames to one veth and do XDP on its peer. This does not work for
the case of macvlan which is based on rx handler.
Actually, for the case of veth, if we implement XDP rx handler for
bridge it can works seamlessly with veth like.
eth0(XDP_PASS) -> [bridge XDP rx handler and ndo_xdp_xmit()] -> veth ---
veth (XDP).
Besides the usage for containers, we can implement macvtap RX handler
which allows a fast packet forwarding to userspace.
> In case where there is no xdp+bpf in final processing, the frames are
> converted to skb and performance is lost, so in such cases there is no
> need for builtin macvlan acting on raw xdp frames either. Just keep
> existing macvlan acting on skbs.
>
Yes, this is how veth works as well.
Actually, the idea is not limited to macvlan but for all device that is
based on rx handler. Consider the case of bonding, this allows to set a
very simple XDP program on slaves and keep a single main logic XDP
program on the bond instead of duplicating it in all slaves.
Thanks
^ permalink raw reply
* I found a strange place while reading “net/ipv6/reassembly.c”
From: Ttttabcd @ 2018-08-15 4:38 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hello everyone who develops the kernel.
At the beginning I was looking for the source author, but his email address has expired, so I can only come here to ask questions.
The problem is in the /net/ipv6/reassembly.c file, the author is Pedro Roque.
I found some strange places when I read the code for this file (Linux Kernel version 4.18).
In the "/net/ipv6/reassembly.c"
In the function "ip6_frag_queue"
offset = ntohs(fhdr->frag_off) & ~0x7;
end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
if ((unsigned int)end > IPV6_MAXPLEN) {
*prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
return -1;
}
Here the length of the payload is judged.
And in the function "ip6_frag_reasm"
payload_len = ((head->data - skb_network_header(head)) -
sizeof(struct ipv6hdr) + fq->q.len -
sizeof(struct frag_hdr));
if (payload_len > IPV6_MAXPLEN)
goto out_oversize;
......
out_oversize:
net_dbg_ratelimited("ip6_frag_reasm: payload len = %d\n", payload_len);
goto out_fail;
Here also judges the length of the payload.
Judged the payload length twice.
I tested that the code in the label "out_oversize:" does not execute at all, because it has been returned in "ip6_frag_queue".
Unless I comment out the code that judge the payload length in the function "ip6_frag_queue", the code labeled "out_oversize:" can be executed.
So, is this repeated?
^ permalink raw reply
* Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands
From: Dan Carpenter @ 2018-08-15 8:29 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Kees Cook, Security Officers, Kevin Deus, Samuel Ortiz,
David S. Miller, Allen Pais, linux-wireless, Network Development,
LKML
In-Reply-To: <CAJuCfpGSPkB-tk4GHaJeStRph-ibVhUZ119Sfd16xgf9x=qUwQ@mail.gmail.com>
On Tue, Aug 14, 2018 at 03:38:14PM -0700, Suren Baghdasaryan wrote:
> The separate fix for the size of pipes[] array is posted here:
> https://lkml.org/lkml/2018/8/14/1034
> Thanks!
>
That's great! Let's add some bounds checking to nfc_hci_msg_rx_work()
and nfc_hci_recv_from_llc() as well and then we can close the chapter on
these bugs.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Pravin Shelar @ 2018-08-15 7:19 UTC (permalink / raw)
To: Stefano Brivio
Cc: Matteo Croce, Justin Pettit, Greg Rose, Ben Pfaff, netdev,
ovs dev, Jiri Benc, Aaron
In-Reply-To: <20180807153111.53b0da8d@epycfail>
Hi Stefano
On Tue, Aug 7, 2018 at 6:31 AM, Stefano Brivio <sbrivio@redhat.com> wrote:
> Hi Pravin,
>
> On Tue, 31 Jul 2018 16:12:03 -0700
> Pravin Shelar <pshelar@ovn.org> wrote:
>
>> Rather than reducing number of thread down to 1, we could find better
>> number of FDs per port.
>> How about this simple solution:
>> 1. Allocate (N * P) FDs as long as it is under FD limit.
>> 2. If FD limit (-EMFILE) is hit reduce N value by half and repeat step 1.
>
> I still see a few quantitative issues with this approach, other than
> Ben's observation about design (which, by the way, looks entirely
> reasonable to me).
>
> We're talking about a disproportionate amount of sockets in any case.
> We can have up to 2^16 vports here, with 5k vports being rather common,
> and for any reasonable value of N that manages somehow to perturbate the
> distribution of upcalls per thread, we are talking about something well
> in excess of 100k sockets. I think this doesn't really scale.
>
My argument is not about proposed fairness algorithm. It is about cost
of the fairness and I do not see it is addressed in any of the follow
ups. You seems to be worried about memory cost and fairness aspects, I
am worried about CPU cost of the solution.
I think proposed solution is solving the fairness issue but it is also
creating bottleneck in upcall processing. OVS is known to have slower
upcall processing. This patch is adding even more cost to the upcall
handling. The latency of first packet handling is also going up with
this approach.
I revisited the original patch, here is what I see in term of added
cost to existing upcall processing:
1. one "kzalloc(sizeof(*upcall), GFP_ATOMIC);" This involve allocate
and initialize memory
2. copy flow key which is more than 1 KB (upcall->key = *key)
3. Acquire spin_lock_bh dp->upcalls.lock, which would disable bottom
half processing on CPU while waiting for the global lock.
4. Iterate list of queued upcalls, one of objective it is to avoid out
of order packet. But I do not see point of ordering packets from
different streams.
5. signal upcall thread after delay ovs_dp_upcall_delay(). This adds
further to the latency.
6. upcall is then handed over to different thread (context switch),
likely on different CPU.
8. the upcall object is freed on remote CPU.
9. single lock essentially means OVS kernel datapath upcall processing
is single threaded no matter number of cores in system.
I would be interested in how are we going to address these issues.
In example you were talking about netlink fd issue on server with 48
core, how does this solution works when there are 5K ports each
triggering upcall ? Can you benchmark your patch? Do you have
performance numbers for TCP_CRR with and without this patch ? Also
publish latency numbers for this patch. Please turn off megaflow to
exercise upcall handling.
I understand fairness has cost, but we need to find right balance
between performance and fairness. Current fairness scheme is a
lockless algorithm without much computational overhead, did you try to
improve current algorithm so that it uses less number of ports.
> With the current value for N (3/4 * number of threads) this can even get
> close to /proc/sys/fs/file-max on some systems, and there raising the
> number of allowed file descriptors for ovs-vswitchd isn't a solution
> anymore.
>
> I would instead try to address the concerns that you had about the
> original patch adding fairness in the kernel, rather than trying to
> make the issue appear less severe in ovs-vswitchd.
>
> --
> Stefano
^ permalink raw reply
* [PATCH v3 net-next] veth: Free queues on link delete
From: Toshiaki Makita @ 2018-08-15 8:07 UTC (permalink / raw)
To: David S. Miller; +Cc: Toshiaki Makita, netdev, David Ahern, dsahern
David Ahern reported memory leak in veth.
=======================================================================
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8800354d5c00 (size 1024):
comm "ip", pid 836, jiffies 4294722952 (age 25.904s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x147/0x3ac [veth]
...
unreferenced object 0xffff88002e009c00 (size 1024):
comm "ip", pid 836, jiffies 4294722958 (age 25.898s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x219/0x3ac [veth]
=======================================================================
veth_rq allocated in veth_newlink() was not freed on dellink.
We need to free up them after veth_close() so that any packets will not
reference the queues afterwards. Thus free them in veth_dev_free() in
the same way as freeing stats structure (vstats).
Also move queues allocation to veth_dev_init() to be in line with stats
allocation.
Fixes: 638264dc90227 ("veth: Support per queue XDP ring")
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
This is a fix for a bug which exists only in net-next.
Let me know if I should wait for -next merging into net or reopen of -next.
drivers/net/veth.c | 70 +++++++++++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index e3202af..8d679c8 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -789,16 +789,48 @@ static int is_valid_veth_mtu(int mtu)
return mtu >= ETH_MIN_MTU && mtu <= ETH_MAX_MTU;
}
+static int veth_alloc_queues(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ int i;
+
+ priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL);
+ if (!priv->rq)
+ return -ENOMEM;
+
+ for (i = 0; i < dev->num_rx_queues; i++)
+ priv->rq[i].dev = dev;
+
+ return 0;
+}
+
+static void veth_free_queues(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ kfree(priv->rq);
+}
+
static int veth_dev_init(struct net_device *dev)
{
+ int err;
+
dev->vstats = netdev_alloc_pcpu_stats(struct pcpu_vstats);
if (!dev->vstats)
return -ENOMEM;
+
+ err = veth_alloc_queues(dev);
+ if (err) {
+ free_percpu(dev->vstats);
+ return err;
+ }
+
return 0;
}
static void veth_dev_free(struct net_device *dev)
{
+ veth_free_queues(dev);
free_percpu(dev->vstats);
}
@@ -1040,31 +1072,13 @@ static int veth_validate(struct nlattr *tb[], struct nlattr *data[],
return 0;
}
-static int veth_alloc_queues(struct net_device *dev)
-{
- struct veth_priv *priv = netdev_priv(dev);
-
- priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL);
- if (!priv->rq)
- return -ENOMEM;
-
- return 0;
-}
-
-static void veth_free_queues(struct net_device *dev)
-{
- struct veth_priv *priv = netdev_priv(dev);
-
- kfree(priv->rq);
-}
-
static struct rtnl_link_ops veth_link_ops;
static int veth_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[],
struct netlink_ext_ack *extack)
{
- int err, i;
+ int err;
struct net_device *peer;
struct veth_priv *priv;
char ifname[IFNAMSIZ];
@@ -1117,12 +1131,6 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
return PTR_ERR(peer);
}
- err = veth_alloc_queues(peer);
- if (err) {
- put_net(net);
- goto err_peer_alloc_queues;
- }
-
if (!ifmp || !tbp[IFLA_ADDRESS])
eth_hw_addr_random(peer);
@@ -1151,10 +1159,6 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
* should be re-allocated
*/
- err = veth_alloc_queues(dev);
- if (err)
- goto err_alloc_queues;
-
if (tb[IFLA_ADDRESS] == NULL)
eth_hw_addr_random(dev);
@@ -1174,28 +1178,20 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
*/
priv = netdev_priv(dev);
- for (i = 0; i < dev->real_num_rx_queues; i++)
- priv->rq[i].dev = dev;
rcu_assign_pointer(priv->peer, peer);
priv = netdev_priv(peer);
- for (i = 0; i < peer->real_num_rx_queues; i++)
- priv->rq[i].dev = peer;
rcu_assign_pointer(priv->peer, dev);
return 0;
err_register_dev:
- veth_free_queues(dev);
-err_alloc_queues:
/* nothing to do */
err_configure_peer:
unregister_netdevice(peer);
return err;
err_register_peer:
- veth_free_queues(peer);
-err_peer_alloc_queues:
free_netdev(peer);
return err;
}
--
1.8.3.1
^ permalink raw reply related
* [iproute PATCH 3/4] Merge common code for conditionally colored output
From: Phil Sutter @ 2018-08-15 9:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815090610.16646-1-phil@nwl.cc>
Instead of calling enable_color() conditionally with identical check in
three places, introduce check_enable_color() which does it in one place.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
bridge/bridge.c | 3 +--
include/color.h | 1 +
ip/ip.c | 3 +--
lib/color.c | 9 +++++++++
tc/tc.c | 3 +--
5 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 289a157d37f03..451d684e0bcfd 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -200,8 +200,7 @@ main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
diff --git a/include/color.h b/include/color.h
index c80359d3e2e95..4f2c918db7e43 100644
--- a/include/color.h
+++ b/include/color.h
@@ -13,6 +13,7 @@ enum color_attr {
};
void enable_color(void);
+int check_enable_color(int color, int json);
void set_color_palette(void);
int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...);
enum color_attr ifa_family_color(__u8 ifa_family);
diff --git a/ip/ip.c b/ip/ip.c
index 71d5170c0cc23..38eac5ec1e17d 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -304,8 +304,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
diff --git a/lib/color.c b/lib/color.c
index da1f516cb2492..edf96e5c6ecd7 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -77,6 +77,15 @@ void enable_color(void)
set_color_palette();
}
+int check_enable_color(int color, int json)
+{
+ if (color && !json) {
+ enable_color();
+ return 0;
+ }
+ return 1;
+}
+
void set_color_palette(void)
{
char *p = getenv("COLORFGBG");
diff --git a/tc/tc.c b/tc/tc.c
index 3bb893756f40e..e775550174473 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -515,8 +515,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color && !json)
- enable_color();
+ check_enable_color(color, json);
if (batch_file)
return batch(batch_file);
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 2/4] bridge: Fix check for colored output
From: Phil Sutter @ 2018-08-15 9:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815090610.16646-1-phil@nwl.cc>
There is no point in calling enable_color() conditionally if it was
already called for each time '-color' flag was parsed. Align the
algorithm with that in ip and tc by actually making use of 'color'
variable.
Fixes: e9625d6aead11 ("Merge branch 'iproute2-master' into iproute2-next")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
bridge/bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 7fcfe1116f6e5..289a157d37f03 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -174,7 +174,7 @@ main(int argc, char **argv)
if (netns_switch(argv[1]))
exit(-1);
} else if (matches(opt, "-color") == 0) {
- enable_color();
+ ++color;
} else if (matches(opt, "-compressvlans") == 0) {
++compress_vlans;
} else if (matches(opt, "-force") == 0) {
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 0/4] A bunch of fixes regarding colored output
From: Phil Sutter @ 2018-08-15 9:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas
This series contains fixes for conditionally colored output in patches 1
and 2. Patch 3 merges the common conditionals from ip, tc and bridge
tools. Patch 4 then adds a further restriction to colored output to
prevent garbled output when redirecting into a file.
Phil Sutter (4):
tc: Fix typo in check for colored output
bridge: Fix check for colored output
Merge common code for conditionally colored output
lib: Enable colored output only for TTYs
bridge/bridge.c | 5 ++---
include/color.h | 1 +
ip/ip.c | 3 +--
lib/color.c | 10 ++++++++++
tc/tc.c | 3 +--
5 files changed, 15 insertions(+), 7 deletions(-)
--
2.18.0
^ permalink raw reply
* [iproute PATCH 1/4] tc: Fix typo in check for colored output
From: Phil Sutter @ 2018-08-15 9:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815090610.16646-1-phil@nwl.cc>
The check used binary instead of boolean AND, which means colored output
was enabled only if the number of specified '-color' flags was odd.
Fixes: 2d165c0811058 ("tc: implement color output")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/tc.c b/tc/tc.c
index 3bb5910ffac52..3bb893756f40e 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -515,7 +515,7 @@ int main(int argc, char **argv)
_SL_ = oneline ? "\\" : "\n";
- if (color & !json)
+ if (color && !json)
enable_color();
if (batch_file)
--
2.18.0
^ permalink raw reply related
* [iproute PATCH 4/4] lib: Enable colored output only for TTYs
From: Phil Sutter @ 2018-08-15 9:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Till Maas
In-Reply-To: <20180815090610.16646-1-phil@nwl.cc>
Add an additional prerequisite to check_enable_color() to make sure
stdout actually points to an open TTY device. Otherwise calls like
| ip -color a s >/tmp/foo
will print color escape sequences into that file.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
lib/color.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/color.c b/lib/color.c
index edf96e5c6ecd7..500ba09682697 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -3,6 +3,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <linux/if.h>
@@ -79,7 +80,7 @@ void enable_color(void)
int check_enable_color(int color, int json)
{
- if (color && !json) {
+ if (color && !json && isatty(fileno(stdout))) {
enable_color();
return 0;
}
--
2.18.0
^ permalink raw reply related
* [iproute PATCH] man: ip-route: Clarify referenced versions are Linux ones
From: Phil Sutter @ 2018-08-15 9:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Versioning scheme of Linux and iproute2 is similar, therefore the
referenced kernel versions are likely to confuse readers. Clarify this
by prefixing each kernel version by 'Linux' prefix.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/ip-route.8.in | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index b21a847241427..a33ce1f0f4006 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -483,43 +483,43 @@ seconds and ms, msec or msecs to specify milliseconds.
.TP
-.BI rttvar " TIME " "(2.3.15+ only)"
+.BI rttvar " TIME " "(Linux 2.3.15+ only)"
the initial RTT variance estimate. Values are specified as with
.BI rtt
above.
.TP
-.BI rto_min " TIME " "(2.6.23+ only)"
+.BI rto_min " TIME " "(Linux 2.6.23+ only)"
the minimum TCP Retransmission TimeOut to use when communicating with this
destination. Values are specified as with
.BI rtt
above.
.TP
-.BI ssthresh " NUMBER " "(2.3.15+ only)"
+.BI ssthresh " NUMBER " "(Linux 2.3.15+ only)"
an estimate for the initial slow start threshold.
.TP
-.BI cwnd " NUMBER " "(2.3.15+ only)"
+.BI cwnd " NUMBER " "(Linux 2.3.15+ only)"
the clamp for congestion window. It is ignored if the
.B lock
flag is not used.
.TP
-.BI initcwnd " NUMBER " "(2.5.70+ only)"
+.BI initcwnd " NUMBER " "(Linux 2.5.70+ only)"
the initial congestion window size for connections to this destination.
Actual window size is this value multiplied by the MSS
(``Maximal Segment Size'') for same connection. The default is
zero, meaning to use the values specified in RFC2414.
.TP
-.BI initrwnd " NUMBER " "(2.6.33+ only)"
+.BI initrwnd " NUMBER " "(Linux 2.6.33+ only)"
the initial receive window size for connections to this destination.
Actual window size is this value multiplied by the MSS of the connection.
The default value is zero, meaning to use Slow Start value.
.TP
-.BI features " FEATURES " (3.18+ only)
+.BI features " FEATURES " (Linux 3.18+ only)
Enable or disable per-route features. Only available feature at this
time is
.B ecn
@@ -531,17 +531,17 @@ also be used even if the
sysctl is set to 0.
.TP
-.BI quickack " BOOL " "(3.11+ only)"
+.BI quickack " BOOL " "(Linux 3.11+ only)"
Enable or disable quick ack for connections to this destination.
.TP
-.BI fastopen_no_cookie " BOOL " "(4.15+ only)"
+.BI fastopen_no_cookie " BOOL " "(Linux 4.15+ only)"
Enable TCP Fastopen without a cookie for connections to this destination.
.TP
-.BI congctl " NAME " "(3.20+ only)"
+.BI congctl " NAME " "(Linux 3.20+ only)"
.TP
-.BI "congctl lock" " NAME " "(3.20+ only)"
+.BI "congctl lock" " NAME " "(Linux 3.20+ only)"
Sets a specific TCP congestion control algorithm only for a given destination.
If not specified, Linux keeps the current global default TCP congestion control
algorithm, or the one set from the application. If the modifier
@@ -554,14 +554,14 @@ control algorithm for that destination, thus it will be enforced/guaranteed to
use the proposed algorithm.
.TP
-.BI advmss " NUMBER " "(2.3.15+ only)"
+.BI advmss " NUMBER " "(Linux 2.3.15+ only)"
the MSS ('Maximal Segment Size') to advertise to these
destinations when establishing TCP connections. If it is not given,
Linux uses a default value calculated from the first hop device MTU.
(If the path to these destination is asymmetric, this guess may be wrong.)
.TP
-.BI reordering " NUMBER " "(2.3.15+ only)"
+.BI reordering " NUMBER " "(Linux 2.3.15+ only)"
Maximal reordering on the path to this destination.
If it is not given, Linux uses the value selected with
.B sysctl
@@ -782,7 +782,7 @@ is a set of encapsulation attributes specific to the
.IR SEG6_ACTION " [ "
.IR SEG6_ACTION_PARAM " ] "
- Operation to perform on matching packets.
-The following actions are currently supported (\fB4.14+ only\fR).
+The following actions are currently supported (\fBLinux 4.14+ only\fR).
.in +2
.B End
@@ -830,7 +830,7 @@ address is set as described in \fBip-sr\fR(8).
.in -8
.TP
-.BI expires " TIME " "(4.4+ only)"
+.BI expires " TIME " "(Linux 4.4+ only)"
the route will be deleted after the expires time.
.B Only
support IPv6 at present.
--
2.18.0
^ permalink raw reply related
* KINDLY REPLY stemlightresources@gmail.com URGENTLY
From: STEMLIGHTRESOURCES @ 2018-08-15 11:00 UTC (permalink / raw)
To: Recipients
KINDLY REPLY stemlightresources@gmail.com URGENTLY
^ permalink raw reply
* Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs
From: Lad, Prabhakar @ 2018-08-15 13:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: Uwe Kleine-König, a.fatoum, David S. Miller, Nicolas Ferre,
netdev, mdf, stable, Sascha Hauer, brad.mouring, Florian Fainelli
In-Reply-To: <20180815023233.GD11610@lunn.ch>
Hi,
On Wed, Aug 15, 2018 at 3:35 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote:
> > Hello Ahmad,
> >
> >
> > On Tue, Aug 14, 2018 at 04:12:40PM +0200, Ahmad Fatoum wrote:
> > > The referenced commit broke initializing macb on the EVB-KSZ9477 eval board.
> > > There, of_mdiobus_register was called even for the fixed-link representing
> > > the SPI-connected switch PHY, with the result that the driver attempts to
> > > enumerate PHYs on a non-existent MDIO bus:
> > >
I ran into a similar problem on v14.4 for davinci_mdio I had to patch
it with [1].
The cpsw has 2 phys one phy is connected to KSZ9031 and other to
ksz9897 Ethernet switch
which is treated as a fixed phy with no mdio lines because of which
mdio_read/write failed.
This didn’t happen in v4.9.x something in core has changed ?
[1]
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 3e84107..197baa6 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -245,6 +245,13 @@ static int davinci_mdio_read(struct mii_bus *bus,
int phy_id, int phy_reg)
u32 reg;
int ret;
+ if (phy_id == 2)
+ return 0;
+
if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
return -EINVAL;
@@ -289,6 +296,13 @@ static int davinci_mdio_write(struct mii_bus
*bus, int phy_id,
u32 reg;
int ret;
+ if (phy_id == 2)
+ return 0;
+
if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
return -EINVAL;
Cheers,
--Prabhakar Lad
^ permalink raw reply related
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