* Re: [PATCH net] net: socionext: Fix a signedness bug in ave_probe()
From: David Miller @ 2019-09-27 8:14 UTC (permalink / raw)
To: dan.carpenter; +Cc: hayashi.kunihiko, netdev, kernel-janitors
In-Reply-To: <20190925105750.GG3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 13:57:50 +0300
> The "phy_mode" variable is an enum and in this context GCC treats it as
> an unsigned int so the error handling is never triggered.
>
> Fixes: 4c270b55a5af ("net: ethernet: socionext: add AVE ethernet driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* [PATCH v2 0/2] net: phy: mscc-miim: Adjustments for mscc_miim_probe()
From: Markus Elfring @ 2019-09-27 8:14 UTC (permalink / raw)
To: netdev, Alexandre Belloni, Andrew Lunn, Florian Fainelli,
David S. Miller, Heiner Kallweit
Cc: LKML, kernel-janitors
In-Reply-To: <20190926193239.GC6825@piout.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 27 Sep 2019 09:48:09 +0200
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Use devm_platform_ioremap_resource()
Move the setting of mii_bus structure members
drivers/net/phy/mdio-mscc-miim.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
--
2.23.0
^ permalink raw reply
* Re: [PATCH net] net: stmmac: dwmac-meson8b: Fix signedness bug in probe
From: David Miller @ 2019-09-27 8:15 UTC (permalink / raw)
To: dan.carpenter
Cc: peppe.cavallaro, martin.blumenstingl, alexandre.torgue, joabreu,
khilman, mcoquelin.stm32, netdev, linux-stm32, kernel-janitors
In-Reply-To: <20190925105822.GH3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 13:58:22 +0300
> The "dwmac->phy_mode" is an enum and in this context GCC treats it as
> an unsigned int so the error handling is never triggered.
>
> Fixes: 566e82516253 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* [PATCH v2 1/2] net: phy: mscc-miim: Use devm_platform_ioremap_resource() in mscc_miim_probe()
From: Markus Elfring @ 2019-09-27 8:16 UTC (permalink / raw)
To: netdev, Alexandre Belloni, Andrew Lunn, Florian Fainelli,
David S. Miller, Heiner Kallweit
Cc: LKML, kernel-janitors
In-Reply-To: <8fbfa9ad-182a-e2ce-e91f-7350523e33c7@web.de>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 27 Sep 2019 09:24:03 +0200
Simplify this function implementation a bit by using
a known wrapper function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2:
Alexandre Belloni pointed the need for adjustments out.
https://lore.kernel.org/r/20190926193239.GC6825@piout.net/
* The mapping of internal phy registers should be treated as optional.
* An other prefix would be preferred in the commit subject.
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c
index badbc99bedd3..aabb13982251 100644
--- a/drivers/net/phy/mdio-mscc-miim.c
+++ b/drivers/net/phy/mdio-mscc-miim.c
@@ -120,10 +120,6 @@ static int mscc_miim_probe(struct platform_device *pdev)
struct mscc_miim_dev *dev;
int ret;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -ENODEV;
-
bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*dev));
if (!bus)
return -ENOMEM;
@@ -136,7 +132,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
bus->parent = &pdev->dev;
dev = bus->priv;
- dev->regs = devm_ioremap_resource(&pdev->dev, res);
+ dev->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(dev->regs)) {
dev_err(&pdev->dev, "Unable to map MIIM registers\n");
return PTR_ERR(dev->regs);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH net] net: axienet: fix a signedness bug in probe
From: David Miller @ 2019-09-27 8:17 UTC (permalink / raw)
To: dan.carpenter
Cc: radhey.shyam.pandey, alvaro.gamez, michal.simek, linux, netdev,
kernel-janitors
In-Reply-To: <20190925105911.GI3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 13:59:11 +0300
> The "lp->phy_mode" is an enum but in this context GCC treats it as an
> unsigned int so the error handling is never triggered.
>
> Fixes: ee06b1728b95 ("net: axienet: add support for standard phy-mode binding")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* [PATCH v2 2/2] net: phy: mscc-miim: Move the setting of mii_bus structure members in mscc_miim_probe()
From: Markus Elfring @ 2019-09-27 8:18 UTC (permalink / raw)
To: netdev, Alexandre Belloni, Andrew Lunn, Florian Fainelli,
David S. Miller, Heiner Kallweit
Cc: LKML, kernel-janitors
In-Reply-To: <8fbfa9ad-182a-e2ce-e91f-7350523e33c7@web.de>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 27 Sep 2019 09:30:14 +0200
Move the modification of some members in the data structure “mii_bus”
for the local variable “bus” directly before the call of
the function “of_mdiobus_register” so that this change will be performed
only after previous resource allocations succeeded.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2:
This suggestion was repeated based on the change for the previous update step.
drivers/net/phy/mdio-mscc-miim.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c
index aabb13982251..a270f83bb207 100644
--- a/drivers/net/phy/mdio-mscc-miim.c
+++ b/drivers/net/phy/mdio-mscc-miim.c
@@ -124,13 +124,6 @@ static int mscc_miim_probe(struct platform_device *pdev)
if (!bus)
return -ENOMEM;
- bus->name = "mscc_miim";
- bus->read = mscc_miim_read;
- bus->write = mscc_miim_write;
- bus->reset = mscc_miim_reset;
- snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
- bus->parent = &pdev->dev;
-
dev = bus->priv;
dev->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(dev->regs)) {
@@ -147,6 +140,12 @@ static int mscc_miim_probe(struct platform_device *pdev)
}
}
+ bus->name = "mscc_miim";
+ bus->read = mscc_miim_read;
+ bus->write = mscc_miim_write;
+ bus->reset = mscc_miim_reset;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+ bus->parent = &pdev->dev;
ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] lib: dimlib: fix help text typos
From: Uwe Kleine-König @ 2019-09-27 8:19 UTC (permalink / raw)
To: Randy Dunlap, netdev@vger.kernel.org, David Miller
Cc: Tal Gilboa, Saeed Mahameed
In-Reply-To: <445cadc0-8b22-957f-47f6-2e6250124ae3@infradead.org>
[-- Attachment #1.1: Type: text/plain, Size: 434 bytes --]
On 9/26/19 2:20 AM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix help text typos for DIMLIB.
>
> Fixes: 4f75da3666c0 ("linux/dim: Move implementation to .c files")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Acked-by: Uwe Kleine-König <uwe@kleine-koenig.org>
Thanks for picking that up and improve my patch.
Best regards
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] of: mdio: Fix a signedness bug in of_phy_get_and_connect()
From: David Miller @ 2019-09-27 8:20 UTC (permalink / raw)
To: dan.carpenter
Cc: andrew, lidongpo, f.fainelli, hkallweit1, robh+dt, frowand.list,
netdev, devicetree, linux-kernel, kernel-janitors
In-Reply-To: <20190925110100.GL3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 14:01:00 +0300
> The "iface" variable is an enum and in this context GCC treats it as
> an unsigned int so the error handling is never triggered.
>
> Fixes: b78624125304 ("of_mdio: Abstract a general interface for phy connect")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: nixge: Fix a signedness bug in nixge_probe()
From: David Miller @ 2019-09-27 8:21 UTC (permalink / raw)
To: dan.carpenter; +Cc: mdf, andrew, alex.williams, mcgrof, netdev, kernel-janitors
In-Reply-To: <20190925110524.GP3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 14:05:24 +0300
> The "priv->phy_mode" is an enum and in this context GCC will treat it
> as an unsigned int so it can never be less than zero.
>
> Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: ethernet: stmmac: Fix signedness bug in ipq806x_gmac_of_parse()
From: David Miller @ 2019-09-27 8:22 UTC (permalink / raw)
To: dan.carpenter
Cc: peppe.cavallaro, mathieu, alexandre.torgue, joabreu,
mcoquelin.stm32, netdev, linux-stm32, kernel-janitors
In-Reply-To: <20190925110554.GQ3264@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 25 Sep 2019 14:05:54 +0300
> The "gmac->phy_mode" variable is an enum and in this context GCC will
> treat it as an unsigned int so the error handling will never be
> triggered.
>
> Fixes: b1c17215d718 ("stmmac: add ipq806x glue layer")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] tcp: better handle TCP_USER_TIMEOUT in SYN_SENT state
From: Jonathan Maxwell @ 2019-09-27 8:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Yuchung Cheng,
Marek Majkowski
In-Reply-To: <20190926224251.249797-1-edumazet@google.com>
Acked-by: Jon Maxwell <jmaxwell37@gmail.com>
Thanks for fixing that Eric.
On Fri, Sep 27, 2019 at 8:42 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Yuchung Cheng and Marek Majkowski independently reported a weird
> behavior of TCP_USER_TIMEOUT option when used at connect() time.
>
> When the TCP_USER_TIMEOUT is reached, tcp_write_timeout()
> believes the flow should live, and the following condition
> in tcp_clamp_rto_to_user_timeout() programs one jiffie timers :
>
> remaining = icsk->icsk_user_timeout - elapsed;
> if (remaining <= 0)
> return 1; /* user timeout has passed; fire ASAP */
>
> This silly situation ends when the max syn rtx count is reached.
>
> This patch makes sure we honor both TCP_SYNCNT and TCP_USER_TIMEOUT,
> avoiding these spurious SYN packets.
>
> Fixes: b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout() helper to improve accuracy")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Yuchung Cheng <ycheng@google.com>
> Reported-by: Marek Majkowski <marek@cloudflare.com>
> Cc: Jon Maxwell <jmaxwell37@gmail.com>
> Link: https://marc.info/?l=linux-netdev&m=156940118307949&w=2
> ---
> net/ipv4/tcp_timer.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index dbd9d2d0ee63aa46ad2dda417da6ec9409442b77..40de2d2364a1eca14c259d77ebed361d17829eb9 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -210,7 +210,7 @@ static int tcp_write_timeout(struct sock *sk)
> struct inet_connection_sock *icsk = inet_csk(sk);
> struct tcp_sock *tp = tcp_sk(sk);
> struct net *net = sock_net(sk);
> - bool expired, do_reset;
> + bool expired = false, do_reset;
> int retry_until;
>
> if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
> @@ -242,9 +242,10 @@ static int tcp_write_timeout(struct sock *sk)
> if (tcp_out_of_resources(sk, do_reset))
> return 1;
> }
> + }
> + if (!expired)
> expired = retransmits_timed_out(sk, retry_until,
> icsk->icsk_user_timeout);
> - }
> tcp_fastopen_active_detect_blackhole(sk, expired);
>
> if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))
> --
> 2.23.0.444.g18eeb5a265-goog
>
^ permalink raw reply
* Re: [PATCH net] kcm: disable preemption in kcm_parse_func_strparser()
From: David Miller @ 2019-09-27 8:28 UTC (permalink / raw)
To: edumazet; +Cc: ast, daniel, netdev, eric.dumazet, bpf, syzkaller
In-Reply-To: <20190924192934.212317-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 24 Sep 2019 12:29:34 -0700
> After commit a2c11b034142 ("kcm: use BPF_PROG_RUN")
> syzbot easily triggers the warning in cant_sleep().
>
> As explained in commit 6cab5e90ab2b ("bpf: run bpf programs
> with preemption disabled") we need to disable preemption before
> running bpf programs.
...
> Fixes: a2c11b034142 ("kcm: use BPF_PROG_RUN")
> Fixes: 6cab5e90ab2b ("bpf: run bpf programs with preemption disabled")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] sch_netem: fix rcu splat in netem_enqueue()
From: David Miller @ 2019-09-27 8:30 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, syzkaller
In-Reply-To: <20190924201126.77301-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 24 Sep 2019 13:11:26 -0700
> qdisc_root() use from netem_enqueue() triggers a lockdep warning.
>
> __dev_queue_xmit() uses rcu_read_lock_bh() which is
> not equivalent to rcu_read_lock() + local_bh_disable_bh as far
> as lockdep is concerned.
...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied.
^ permalink raw reply
* Re: [PATCH] vhost: introduce mdev based hardware backend
From: Jason Wang @ 2019-09-27 8:31 UTC (permalink / raw)
To: Tiwei Bie
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <20190927080410.GA22568@___>
On 2019/9/27 下午4:04, Tiwei Bie wrote:
> On Fri, Sep 27, 2019 at 03:14:42PM +0800, Jason Wang wrote:
>> On 2019/9/27 下午12:54, Tiwei Bie wrote:
>>>>> +
>>>>> + /*
>>>>> + * In vhost-mdev, userspace should pass ring addresses
>>>>> + * in guest physical addresses when IOMMU is disabled or
>>>>> + * IOVAs when IOMMU is enabled.
>>>>> + */
>>>> A question here, consider we're using noiommu mode. If guest physical
>>>> address is passed here, how can a device use that?
>>>>
>>>> I believe you meant "host physical address" here? And it also have the
>>>> implication that the HPA should be continuous (e.g using hugetlbfs).
>>> The comment is talking about the virtual IOMMU (i.e. iotlb in vhost).
>>> It should be rephrased to cover the noiommu case as well. Thanks for
>>> spotting this.
>>
>> So the question still, if GPA is passed how can it be used by the
>> virtio-mdev device?
> Sorry if I didn't make it clear..
> Of course, GPA can't be passed in noiommu mode.
I see.
Thanks for the confirmation.
>
>
>> Thanks
>>
^ permalink raw reply
* Re: [PATCH net] net: dsa: qca8k: Fix port enable for CPU port
From: David Miller @ 2019-09-27 8:32 UTC (permalink / raw)
To: andrew; +Cc: netdev, vivien.didelot, michal.vokac
In-Reply-To: <20190925004707.1799-1-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 25 Sep 2019 02:47:07 +0200
> The CPU port does not have a PHY connected to it. So calling
> phy_support_asym_pause() results in an Opps. As with other DSA
> drivers, add a guard that the port is a user port.
>
> Reported-by: Michal Vokáč <michal.vokac@ysoft.com>
> Fixes: 0394a63acfe2 ("net: dsa: enable and disable all ports")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Applied.
^ permalink raw reply
* Re: [PATCH net v2] vsock: Fix a lockdep warning in __vsock_release()
From: Stefano Garzarella @ 2019-09-27 8:32 UTC (permalink / raw)
To: Dexuan Cui
Cc: davem@davemloft.net, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, stefanha@redhat.com,
gregkh@linuxfoundation.org, arnd@arndb.de, deepa.kernel@gmail.com,
ytht.net@gmail.com, tglx@linutronix.de, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
Michael Kelley, jhansen@vmware.com
In-Reply-To: <PU1P153MB01698C46C9348B9762D5E122BF810@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
On Fri, Sep 27, 2019 at 05:37:20AM +0000, Dexuan Cui wrote:
> > From: linux-hyperv-owner@vger.kernel.org
> > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Stefano Garzarella
> > Sent: Thursday, September 26, 2019 12:48 AM
> >
> > Hi Dexuan,
> >
> > On Thu, Sep 26, 2019 at 01:11:27AM +0000, Dexuan Cui wrote:
> > > ...
> > > NOTE: I only tested the code on Hyper-V. I can not test the code for
> > > virtio socket, as I don't have a KVM host. :-( Sorry.
> > >
> > > @Stefan, @Stefano: please review & test the patch for virtio socket,
> > > and let me know if the patch breaks anything. Thanks!
> >
> > Comment below, I'll test it ASAP!
>
> Stefano, Thank you!
>
> BTW, this is how I tested the patch:
> 1. write a socket server program in the guest. The program calls listen()
> and then calls sleep(10000 seconds). Note: accept() is not called.
>
> 2. create some connections to the server program in the guest.
>
> 3. kill the server program by Ctrl+C, and "dmesg" will show the scary
> call-trace, if the kernel is built with
> CONFIG_LOCKDEP=y
> CONFIG_LOCKDEP_SUPPORT=y
>
> 4. Apply the patch, do the same test and we should no longer see the call-trace.
Thanks very useful! I'll follow these steps!
>
> > > - lock_sock(sk);
> > > + /* When "level" is 2, use the nested version to avoid the
> > > + * warning "possible recursive locking detected".
> > > + */
> > > + if (level == 1)
> > > + lock_sock(sk);
> >
> > Since lock_sock() calls lock_sock_nested(sk, 0), could we use directly
> > lock_sock_nested(sk, level) with level = 0 in vsock_release() and
> > level = SINGLE_DEPTH_NESTING here in the while loop?
> >
> > Thanks,
> > Stefano
>
> IMHO it's better to make the lock usage more explicit, as the patch does.
>
> lock_sock_nested(sk, level) or lock_sock_nested(sk, 0) seems a little
> odd to me. But I'm open to your suggestion: if any of the network
> maintainers, e.g. davem, also agrees with you, I'll change the code
> as you suggested. :-)
Sure!
Just to be clear, I'm proposing this (plus the changes in the transports
and yours useful comments):
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -638,7 +638,7 @@ struct sock *__vsock_create(struct net *net,
}
EXPORT_SYMBOL_GPL(__vsock_create);
-static void __vsock_release(struct sock *sk)
+static void __vsock_release(struct sock *sk, int level)
{
if (sk) {
struct sk_buff *skb;
@@ -650,7 +650,7 @@ static void __vsock_release(struct sock *sk)
transport->release(vsk);
- lock_sock(sk);
+ lock_sock_nested(sk, level);
sock_orphan(sk);
sk->sk_shutdown = SHUTDOWN_MASK;
@@ -659,7 +659,7 @@ static void __vsock_release(struct sock *sk)
/* Clean up any sockets that never were accepted. */
while ((pending = vsock_dequeue_accept(sk)) != NULL) {
- __vsock_release(pending);
+ __vsock_release(pending, SINGLE_DEPTH_NESTING);
sock_put(pending);
}
@@ -708,7 +708,7 @@ EXPORT_SYMBOL_GPL(vsock_stream_has_space);
static int vsock_release(struct socket *sock)
{
- __vsock_release(sock->sk);
+ __vsock_release(sock->sk, 0);
sock->sk = NULL;
sock->state = SS_FREE;
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] net/mlx5: prevent memory leak in mlx5_fpga_conn_create_cq
From: David Miller @ 2019-09-27 8:33 UTC (permalink / raw)
To: navid.emamdoost
Cc: emamd001, smccaman, kjlu, borisp, saeedm, leon, yishaih, maximmi,
eranbe, tariqt, bodong, netdev, linux-rdma, linux-kernel
In-Reply-To: <20190925032038.22943-1-navid.emamdoost@gmail.com>
From: Navid Emamdoost <navid.emamdoost@gmail.com>
Date: Tue, 24 Sep 2019 22:20:34 -0500
> In mlx5_fpga_conn_create_cq if mlx5_vector2eqn fails the allocated
> memory should be released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Saeed, please queue this up.
^ permalink raw reply
* Re: [PATCH V2 6/8] mdev: introduce virtio device and its device ops
From: Jason Wang @ 2019-09-27 8:37 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, linux-s390, linux-kernel, dri-devel, intel-gfx,
intel-gvt-dev, kwankhede, mst, tiwei.bie, virtualization, netdev,
cohuck, maxime.coquelin, cunming.liang, zhihong.wang, rob.miller,
xiao.w.wang, haotian.wang, zhenyuw, zhi.a.wang, jani.nikula,
joonas.lahtinen, rodrigo.vivi, airlied, daniel, farman, pasic,
sebott, oberpar, heiko.carstens, gor, borntraeger, akrowiak,
freude, lingshan.zhu, idos, eperezma, lulu, parav,
christophe.de.dinechin, kevin.tian
In-Reply-To: <20190924170640.1da03bae@x1.home>
On 2019/9/25 上午7:06, Alex Williamson wrote:
> On Tue, 24 Sep 2019 21:53:30 +0800
> Jason Wang<jasowang@redhat.com> wrote:
>
>> This patch implements basic support for mdev driver that supports
>> virtio transport for kernel virtio driver.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>> include/linux/mdev.h | 2 +
>> include/linux/virtio_mdev.h | 145 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 147 insertions(+)
>> create mode 100644 include/linux/virtio_mdev.h
>>
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
>> index 3414307311f1..73ac27b3b868 100644
>> --- a/include/linux/mdev.h
>> +++ b/include/linux/mdev.h
>> @@ -126,6 +126,8 @@ struct mdev_device *mdev_from_dev(struct device *dev);
>>
>> enum {
>> MDEV_ID_VFIO = 1,
>> + MDEV_ID_VIRTIO = 2,
>> + MDEV_ID_VHOST = 3,
> MDEV_ID_VHOST isn't used yet here. Also, given the strong
> interdependence between the class_id and the ops structure, we might
> wand to define them in the same place. Thanks,
>
> Alex
>
Rethink about this, I believe it's better to define the device ops in
their own headers, and one set of device ops could be used for two
classes (e.g both virtio and vhost). And to avoid a duplicated ID
definition. I tend to keep this in the common mdev.h header.
Thanks
^ permalink raw reply
* Re: [PATCH] vhost: introduce mdev based hardware backend
From: Jason Wang @ 2019-09-27 8:47 UTC (permalink / raw)
To: Tiwei Bie
Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <20190927045438.GA17152@___>
On 2019/9/27 下午12:54, Tiwei Bie wrote:
> On Fri, Sep 27, 2019 at 11:46:06AM +0800, Jason Wang wrote:
>> On 2019/9/26 下午12:54, Tiwei Bie wrote:
>>> +
>>> +static long vhost_mdev_start(struct vhost_mdev *m)
>>> +{
>>> + struct mdev_device *mdev = m->mdev;
>>> + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
>>> + struct virtio_mdev_callback cb;
>>> + struct vhost_virtqueue *vq;
>>> + int idx;
>>> +
>>> + ops->set_features(mdev, m->acked_features);
>>> +
>>> + mdev_add_status(mdev, VIRTIO_CONFIG_S_FEATURES_OK);
>>> + if (!(mdev_get_status(mdev) & VIRTIO_CONFIG_S_FEATURES_OK))
>>> + goto reset;
>>> +
>>> + for (idx = 0; idx < m->nvqs; idx++) {
>>> + vq = &m->vqs[idx];
>>> +
>>> + if (!vq->desc || !vq->avail || !vq->used)
>>> + break;
>>> +
>>> + if (ops->set_vq_state(mdev, idx, vq->last_avail_idx))
>>> + goto reset;
>> If we do set_vq_state() in SET_VRING_BASE, we won't need this step here.
> Yeah, I plan to do it in the next version.
>
>>> +
>>> + /*
>>> + * In vhost-mdev, userspace should pass ring addresses
>>> + * in guest physical addresses when IOMMU is disabled or
>>> + * IOVAs when IOMMU is enabled.
>>> + */
>> A question here, consider we're using noiommu mode. If guest physical
>> address is passed here, how can a device use that?
>>
>> I believe you meant "host physical address" here? And it also have the
>> implication that the HPA should be continuous (e.g using hugetlbfs).
> The comment is talking about the virtual IOMMU (i.e. iotlb in vhost).
> It should be rephrased to cover the noiommu case as well. Thanks for
> spotting this.
>
>
>>> +
>>> + switch (cmd) {
>>> + case VHOST_MDEV_SET_STATE:
>>> + r = vhost_set_state(m, argp);
>>> + break;
>>> + case VHOST_GET_FEATURES:
>>> + r = vhost_get_features(m, argp);
>>> + break;
>>> + case VHOST_SET_FEATURES:
>>> + r = vhost_set_features(m, argp);
>>> + break;
>>> + case VHOST_GET_VRING_BASE:
>>> + r = vhost_get_vring_base(m, argp);
>>> + break;
>> Does it mean the SET_VRING_BASE may only take affect after
>> VHOST_MEV_SET_STATE?
> Yeah, in this version, SET_VRING_BASE won't set the base to the
> device directly. But I plan to not delay this anymore in the next
> version to support the SET_STATUS.
>
>>> + default:
>>> + r = vhost_dev_ioctl(&m->dev, cmd, argp);
>>> + if (r == -ENOIOCTLCMD)
>>> + r = vhost_vring_ioctl(&m->dev, cmd, argp);
>>> + }
>>> +
>>> + mutex_unlock(&m->mutex);
>>> + return r;
>>> +}
>>> +
>>> +static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = {
>>> + .name = "vfio-vhost-mdev",
>>> + .open = vhost_mdev_open,
>>> + .release = vhost_mdev_release,
>>> + .ioctl = vhost_mdev_unlocked_ioctl,
>>> +};
>>> +
>>> +static int vhost_mdev_probe(struct device *dev)
>>> +{
>>> + struct mdev_device *mdev = mdev_from_dev(dev);
>>> + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
>>> + struct vhost_mdev *m;
>>> + int nvqs, r;
>>> +
>>> + m = kzalloc(sizeof(*m), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
>>> + if (!m)
>>> + return -ENOMEM;
>>> +
>>> + mutex_init(&m->mutex);
>>> +
>>> + nvqs = ops->get_queue_max(mdev);
>>> + m->nvqs = nvqs;
>> The name could be confusing, get_queue_max() is to get the maximum number of
>> entries for a virtqueue supported by this device.
> OK. It might be better to rename it to something like:
>
> get_vq_num_max()
>
> which is more consistent with the set_vq_num().
>
>> It looks to me that we need another API to query the maximum number of
>> virtqueues supported by the device.
> Yeah.
>
> Thanks,
> Tiwei
One problem here:
Consider if we want to support multiqueue, how did userspace know about
this? Note this information could be fetched from get_config() via a
device specific way, do we want ioctl for accessing that area?
Thanks
^ permalink raw reply
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
From: Stefano Garzarella @ 2019-09-27 8:55 UTC (permalink / raw)
To: Eric Dumazet, Matias Ezequiel Vara Larsen
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <f069a65d-33b9-1fa8-d26e-b76cc51fc7cb@gmail.com>
On Thu, Sep 26, 2019 at 12:33:36PM -0700, Eric Dumazet wrote:
>
>
> On 9/26/19 11:23 AM, Matias Ezequiel Vara Larsen wrote:
> > This patch adds support for MSG_PEEK. In such a case, packets are not
> > removed from the rx_queue and credit updates are not sent.
> >
> > Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
> > ---
> > net/vmw_vsock/virtio_transport_common.c | 50 +++++++++++++++++++++++++++++++--
> > 1 file changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 94cc0fa..938f2ed 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -264,6 +264,50 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
> > }
> >
> > static ssize_t
> > +virtio_transport_stream_do_peek(struct vsock_sock *vsk,
> > + struct msghdr *msg,
> > + size_t len)
> > +{
> > + struct virtio_vsock_sock *vvs = vsk->trans;
> > + struct virtio_vsock_pkt *pkt;
> > + size_t bytes, total = 0;
> > + int err = -EFAULT;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> > + if (total == len)
> > + break;
> > +
> > + bytes = len - total;
> > + if (bytes > pkt->len - pkt->off)
> > + bytes = pkt->len - pkt->off;
> > +
> > + /* sk_lock is held by caller so no one else can dequeue.
> > + * Unlock rx_lock since memcpy_to_msg() may sleep.
> > + */
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
> > + if (err)
> > + goto out;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + total += bytes;
> > + }
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + return total;
> > +
> > +out:
> > + if (total)
> > + err = total;
> > + return err;
> > +}
> >
>
> This seems buggy to me.
>
> virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.
>
> The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.
>
> virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
> before cleaning the packet from the queue.
Good catch!
Maybe we can solve in this way:
list_for_each_entry(pkt, &vvs->rx_queue, list) {
size_t off = pkt->off;
if (total == len)
break;
while (total < len && off < pkt->len) {
/* using 'off' instead of 'pkt->off' */
...
total += bytes;
off += bytes;
}
}
What do you think?
Cheers,
Stefano
^ permalink raw reply
* Re: [RFC PATCH v4 3/5] psci: Add hvc call service for ptp_kvm.
From: Sergei Shtylyov @ 2019-09-27 9:20 UTC (permalink / raw)
To: Jianyong Wu, netdev, yangbo.lu, john.stultz, tglx, pbonzini,
sean.j.christopherson, maz, richardcochran, Mark.Rutland,
Will.Deacon, suzuki.poulose
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Steve.Capper,
Kaly.Xin, justin.he, nd
In-Reply-To: <20190926114212.5322-4-jianyong.wu@arm.com>
Hello!
On 26.09.2019 14:42, Jianyong Wu wrote:
> This patch is the base of ptp_kvm for arm64.
> ptp_kvm modules will call hvc to get this service.
> The service offers real time and counter cycle of host for guest.
>
> Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> ---
> include/linux/arm-smccc.h | 12 ++++++++++++
> virt/kvm/arm/psci.c | 18 ++++++++++++++++++
> 2 files changed, 30 insertions(+)
[...]
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 0debf49bf259..3f30fc42a5ca 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
[...]
> @@ -431,6 +433,22 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> break;
> + /*
> + * This will used for virtual ptp kvm clock. three
^ be? ^ T
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] vhost: introduce mdev based hardware backend
From: Michael S. Tsirkin @ 2019-09-27 9:38 UTC (permalink / raw)
To: Jason Wang
Cc: Tiwei Bie, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <05ab395e-6677-e8c3-becf-57bc1529921f@redhat.com>
On Fri, Sep 27, 2019 at 04:47:43PM +0800, Jason Wang wrote:
>
> On 2019/9/27 下午12:54, Tiwei Bie wrote:
> > On Fri, Sep 27, 2019 at 11:46:06AM +0800, Jason Wang wrote:
> > > On 2019/9/26 下午12:54, Tiwei Bie wrote:
> > > > +
> > > > +static long vhost_mdev_start(struct vhost_mdev *m)
> > > > +{
> > > > + struct mdev_device *mdev = m->mdev;
> > > > + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
> > > > + struct virtio_mdev_callback cb;
> > > > + struct vhost_virtqueue *vq;
> > > > + int idx;
> > > > +
> > > > + ops->set_features(mdev, m->acked_features);
> > > > +
> > > > + mdev_add_status(mdev, VIRTIO_CONFIG_S_FEATURES_OK);
> > > > + if (!(mdev_get_status(mdev) & VIRTIO_CONFIG_S_FEATURES_OK))
> > > > + goto reset;
> > > > +
> > > > + for (idx = 0; idx < m->nvqs; idx++) {
> > > > + vq = &m->vqs[idx];
> > > > +
> > > > + if (!vq->desc || !vq->avail || !vq->used)
> > > > + break;
> > > > +
> > > > + if (ops->set_vq_state(mdev, idx, vq->last_avail_idx))
> > > > + goto reset;
> > > If we do set_vq_state() in SET_VRING_BASE, we won't need this step here.
> > Yeah, I plan to do it in the next version.
> >
> > > > +
> > > > + /*
> > > > + * In vhost-mdev, userspace should pass ring addresses
> > > > + * in guest physical addresses when IOMMU is disabled or
> > > > + * IOVAs when IOMMU is enabled.
> > > > + */
> > > A question here, consider we're using noiommu mode. If guest physical
> > > address is passed here, how can a device use that?
> > >
> > > I believe you meant "host physical address" here? And it also have the
> > > implication that the HPA should be continuous (e.g using hugetlbfs).
> > The comment is talking about the virtual IOMMU (i.e. iotlb in vhost).
> > It should be rephrased to cover the noiommu case as well. Thanks for
> > spotting this.
> >
> >
> > > > +
> > > > + switch (cmd) {
> > > > + case VHOST_MDEV_SET_STATE:
> > > > + r = vhost_set_state(m, argp);
> > > > + break;
> > > > + case VHOST_GET_FEATURES:
> > > > + r = vhost_get_features(m, argp);
> > > > + break;
> > > > + case VHOST_SET_FEATURES:
> > > > + r = vhost_set_features(m, argp);
> > > > + break;
> > > > + case VHOST_GET_VRING_BASE:
> > > > + r = vhost_get_vring_base(m, argp);
> > > > + break;
> > > Does it mean the SET_VRING_BASE may only take affect after
> > > VHOST_MEV_SET_STATE?
> > Yeah, in this version, SET_VRING_BASE won't set the base to the
> > device directly. But I plan to not delay this anymore in the next
> > version to support the SET_STATUS.
> >
> > > > + default:
> > > > + r = vhost_dev_ioctl(&m->dev, cmd, argp);
> > > > + if (r == -ENOIOCTLCMD)
> > > > + r = vhost_vring_ioctl(&m->dev, cmd, argp);
> > > > + }
> > > > +
> > > > + mutex_unlock(&m->mutex);
> > > > + return r;
> > > > +}
> > > > +
> > > > +static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = {
> > > > + .name = "vfio-vhost-mdev",
> > > > + .open = vhost_mdev_open,
> > > > + .release = vhost_mdev_release,
> > > > + .ioctl = vhost_mdev_unlocked_ioctl,
> > > > +};
> > > > +
> > > > +static int vhost_mdev_probe(struct device *dev)
> > > > +{
> > > > + struct mdev_device *mdev = mdev_from_dev(dev);
> > > > + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
> > > > + struct vhost_mdev *m;
> > > > + int nvqs, r;
> > > > +
> > > > + m = kzalloc(sizeof(*m), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> > > > + if (!m)
> > > > + return -ENOMEM;
> > > > +
> > > > + mutex_init(&m->mutex);
> > > > +
> > > > + nvqs = ops->get_queue_max(mdev);
> > > > + m->nvqs = nvqs;
> > > The name could be confusing, get_queue_max() is to get the maximum number of
> > > entries for a virtqueue supported by this device.
> > OK. It might be better to rename it to something like:
> >
> > get_vq_num_max()
> >
> > which is more consistent with the set_vq_num().
> >
> > > It looks to me that we need another API to query the maximum number of
> > > virtqueues supported by the device.
> > Yeah.
> >
> > Thanks,
> > Tiwei
>
>
> One problem here:
>
> Consider if we want to support multiqueue, how did userspace know about
> this?
There's a feature bit for this, isn't there?
> Note this information could be fetched from get_config() via a device
> specific way, do we want ioctl for accessing that area?
>
> Thanks
^ permalink raw reply
* [PATCH] net: tap: clean up an indentation issue
From: Colin King @ 2019-09-27 9:40 UTC (permalink / raw)
To: David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a statement that is indented too deeply, remove
the extraneous tab.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/tap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index bcdfb0d88753..a6d63665ad03 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1190,7 +1190,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
kfree_skb(skb);
err:
rcu_read_lock();
- tap = rcu_dereference(q->tap);
+ tap = rcu_dereference(q->tap);
if (tap && tap->count_tx_dropped)
tap->count_tx_dropped(tap);
rcu_read_unlock();
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] vhost: introduce mdev based hardware backend
From: Michael S. Tsirkin @ 2019-09-27 9:41 UTC (permalink / raw)
To: Jason Wang
Cc: Tiwei Bie, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <8ab5a8d9-284d-bba5-803d-08523c0814e1@redhat.com>
On Fri, Sep 27, 2019 at 11:27:12AM +0800, Jason Wang wrote:
>
> On 2019/9/26 下午9:14, Tiwei Bie wrote:
> > On Thu, Sep 26, 2019 at 04:35:18AM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Sep 26, 2019 at 12:54:27PM +0800, Tiwei Bie wrote:
> > [...]
> > > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > > > index 40d028eed645..5afbc2f08fa3 100644
> > > > --- a/include/uapi/linux/vhost.h
> > > > +++ b/include/uapi/linux/vhost.h
> > > > @@ -116,4 +116,12 @@
> > > > #define VHOST_VSOCK_SET_GUEST_CID _IOW(VHOST_VIRTIO, 0x60, __u64)
> > > > #define VHOST_VSOCK_SET_RUNNING _IOW(VHOST_VIRTIO, 0x61, int)
> > > > +/* VHOST_MDEV specific defines */
> > > > +
> > > > +#define VHOST_MDEV_SET_STATE _IOW(VHOST_VIRTIO, 0x70, __u64)
> > > > +
> > > > +#define VHOST_MDEV_S_STOPPED 0
> > > > +#define VHOST_MDEV_S_RUNNING 1
> > > > +#define VHOST_MDEV_S_MAX 2
> > > > +
> > > > #endif
> > > So assuming we have an underlying device that behaves like virtio:
> > I think they are really good questions/suggestions. Thanks!
> >
> > > 1. Should we use SET_STATUS maybe?
> > I like this idea. I will give it a try.
> >
> > > 2. Do we want a reset ioctl?
> > I think it is helpful. If we use SET_STATUS, maybe we
> > can use it to support the reset.
> >
> > > 3. Do we want ability to enable rings individually?
> > I will make it possible at least in the vhost layer.
>
>
> Note the API support e.g set_vq_ready().
virtio spec calls this "enabled" so let's stick to that.
>
> >
> > > 4. Does device need to limit max ring size?
> > > 5. Does device need to limit max number of queues?
> > I think so. It's helpful to have ioctls to report the max
> > ring size and max number of queues.
>
>
> An issue is the max number of queues is done through a device specific way,
> usually device configuration space. This is supported by the transport API,
> but how to expose it to userspace may need more thought.
>
> Thanks
an ioctl for device config? But for v1 I'd be quite happy to just have
a minimal working device with 2 queues.
>
> >
> > Thanks!
^ permalink raw reply
* RE: Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
From: zhangsha (A) @ 2019-09-27 9:43 UTC (permalink / raw)
To: Aleksei Zakharov, Jay Vosburgh; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAJYOGF9TY8WtUscsfJ=qduAw7_1BwU+4iE+eL6cidM=LBL9w+A@mail.gmail.com>
> -----Original Message-----
> From: Aleksei Zakharov [mailto:zaharov@selectel.ru]
> Sent: 2019年9月25日 19:02
> To: Jay Vosburgh <jay.vosburgh@canonical.com>
> Cc: netdev@vger.kernel.org; zhangsha (A) <zhangsha.zhang@huawei.com>
> Subject: Re: Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
>
> ср, 25 сент. 2019 г. в 03:31, Jay Vosburgh <jay.vosburgh@canonical.com>:
> >
> > Алексей Захаров wrote:
> > [...]
> > >Right after reboot one of the slaves hangs with actor port state 71
> > >and partner port state 1.
> > >It doesn't send lacpdu and seems to be broken.
> > >Setting link down and up again fixes slave state.
> > [...]
> >
> > I think I see what failed in the first patch, could you test
> > the following patch? This one is for net-next, so you'd need to again
> > swap slave_err / netdev_err for the Ubuntu 4.15 kernel.
> >
> I've tested new patch. It seems to work. I can't reproduce the bug with this
> patch.
> There are two types of messages when link becomes up:
> First:
> bond-san: EVENT 1 llu 4294895911 slave eth2
> 8021q: adding VLAN 0 to HW filter on device eth2
> bond-san: link status definitely down for interface eth2, disabling it
> mlx4_en: eth2: Link Up
> bond-san: EVENT 4 llu 4294895911 slave eth2
> bond-san: link status up for interface eth2, enabling it in 500 ms
> bond-san: invalid new link 3 on slave eth2
> bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
> Second:
> bond-san: EVENT 1 llu 4295147594 slave eth2
> 8021q: adding VLAN 0 to HW filter on device eth2
> mlx4_en: eth2: Link Up
> bond-san: EVENT 4 llu 4295147594 slave eth2
> bond-san: link status up again after 0 ms for interface eth2
> bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
>
> These messages (especially "invalid new link") look a bit unclear from sysadmin
> point of view.
> But lacp seems to work fine, thank you!
>
I tests the patch by skipping the bond_update_speed_duplex() function when
the bond receiving a 'NETDEV_UP' event, so that I can force the bond entering status
BOND_LINK_FAIL. In this scenario, my lacp seems to work fine.
Messages I got:
[24662.081877] bond0: link status definitely down for interface eth2, disabling it
[24662.081881] bond0: first active interface up!
[24684.153871] bond0: link status up again after 0 ms for interface eth2
[24684.156846] bond0: link status definitely up for interface eth2, 10000 Mbps full duplex
[24684.156851] bond0: first active interface up!
> > diff --git a/drivers/net/bonding/bond_main.c
> > b/drivers/net/bonding/bond_main.c index 931d9d935686..5e248588259a
> > 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -1617,6 +1617,7 @@ int bond_enslave(struct net_device *bond_dev,
> struct net_device *slave_dev,
> > if (bond->params.miimon) {
> > if (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS) {
> > if (bond->params.updelay) {
> > +/*XXX*/slave_info(bond_dev, slave_dev, "BOND_LINK_BACK initial
> > +state\n");
> > bond_set_slave_link_state(new_slave,
> > BOND_LINK_BACK,
> >
> > BOND_SLAVE_NOTIFY_NOW); @@ -2086,8 +2087,7 @@ static int
> bond_miimon_inspect(struct bonding *bond)
> > ignore_updelay = !rcu_dereference(bond->curr_active_slave);
> >
> > bond_for_each_slave_rcu(bond, slave, iter) {
> > - slave->new_link = BOND_LINK_NOCHANGE;
> > - slave->link_new_state = slave->link;
> > + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
> >
> > link_state = bond_check_dev_link(bond, slave->dev, 0);
> >
> > @@ -2096,8 +2096,6 @@ static int bond_miimon_inspect(struct bonding
> *bond)
> > if (link_state)
> > continue;
> >
> > - bond_propose_link_state(slave, BOND_LINK_FAIL);
> > - commit++;
> > slave->delay = bond->params.downdelay;
> > if (slave->delay) {
> > slave_info(bond->dev, slave->dev,
> > "link status down for %sinterface, disabling it in %d ms\n", @@ -2106,6
> +2104,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> > (bond_is_active_slave(slave) ?
> > "active " : "backup ") : "",
> > bond->params.downdelay *
> > bond->params.miimon);
> > + slave->link = BOND_LINK_FAIL;
> > }
> > /*FALLTHRU*/
> > case BOND_LINK_FAIL:
> > @@ -2121,7 +2120,7 @@ static int bond_miimon_inspect(struct bonding
> *bond)
> > }
> >
> > if (slave->delay <= 0) {
> > - slave->new_link = BOND_LINK_DOWN;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_DOWN);
> > commit++;
> > continue;
> > }
> > @@ -2133,15 +2132,13 @@ static int bond_miimon_inspect(struct bonding
> *bond)
> > if (!link_state)
> > continue;
> >
> > - bond_propose_link_state(slave, BOND_LINK_BACK);
> > - commit++;
> > slave->delay = bond->params.updelay;
> > -
> > if (slave->delay) {
> > slave_info(bond->dev, slave->dev, "link status up, enabling
> it in %d ms\n",
> > ignore_updelay ? 0 :
> > bond->params.updelay *
> > bond->params.miimon);
> > + slave->link = BOND_LINK_BACK;
> > }
> > /*FALLTHRU*/
> > case BOND_LINK_BACK:
> > @@ -2158,7 +2155,7 @@ static int bond_miimon_inspect(struct bonding
> *bond)
> > slave->delay = 0;
> >
> > if (slave->delay <= 0) {
> > - slave->new_link = BOND_LINK_UP;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_UP);
> > commit++;
> > ignore_updelay = false;
> > continue; @@ -2196,7 +2193,7 @@ static
> > void bond_miimon_commit(struct bonding *bond)
> > struct slave *slave, *primary;
> >
> > bond_for_each_slave(bond, slave, iter) {
> > - switch (slave->new_link) {
> > + switch (slave->link_new_state) {
> > case BOND_LINK_NOCHANGE:
> > /* For 802.3ad mode, check current slave speed and
> > * duplex again in case its port was disabled
> > after @@ -2268,8 +2265,8 @@ static void bond_miimon_commit(struct
> > bonding *bond)
> >
> > default:
> > slave_err(bond->dev, slave->dev, "invalid new link %d on
> slave\n",
> > - slave->new_link);
> > - slave->new_link = BOND_LINK_NOCHANGE;
> > + slave->link_new_state);
> > + bond_propose_link_state(slave,
> > + BOND_LINK_NOCHANGE);
> >
> > continue;
> > }
> > @@ -2677,13 +2674,13 @@ static void bond_loadbalance_arp_mon(struct
> bonding *bond)
> > bond_for_each_slave_rcu(bond, slave, iter) {
> > unsigned long trans_start =
> > dev_trans_start(slave->dev);
> >
> > - slave->new_link = BOND_LINK_NOCHANGE;
> > + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
> >
> > if (slave->link != BOND_LINK_UP) {
> > if (bond_time_in_interval(bond, trans_start, 1) &&
> > bond_time_in_interval(bond,
> > slave->last_rx, 1)) {
> >
> > - slave->new_link = BOND_LINK_UP;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_UP);
> > slave_state_changed = 1;
> >
> > /* primary_slave has no meaning in
> > round-robin @@ -2708,7 +2705,7 @@ static void
> bond_loadbalance_arp_mon(struct bonding *bond)
> > if (!bond_time_in_interval(bond, trans_start, 2) ||
> > !bond_time_in_interval(bond,
> > slave->last_rx, 2)) {
> >
> > - slave->new_link = BOND_LINK_DOWN;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_DOWN);
> > slave_state_changed = 1;
> >
> > if (slave->link_failure_count <
> > UINT_MAX) @@ -2739,8 +2736,8 @@ static void
> bond_loadbalance_arp_mon(struct bonding *bond)
> > goto re_arm;
> >
> > bond_for_each_slave(bond, slave, iter) {
> > - if (slave->new_link != BOND_LINK_NOCHANGE)
> > - slave->link = slave->new_link;
> > + if (slave->link_new_state != BOND_LINK_NOCHANGE)
> > + slave->link = slave->link_new_state;
> > }
> >
> > if (slave_state_changed) { @@ -2763,9 +2760,9 @@
> > static void bond_loadbalance_arp_mon(struct bonding *bond) }
> >
> > /* Called to inspect slaves for active-backup mode ARP monitor link
> > state
> > - * changes. Sets new_link in slaves to specify what action should
> > take
> > - * place for the slave. Returns 0 if no changes are found, >0 if
> > changes
> > - * to link states must be committed.
> > + * changes. Sets proposed link state in slaves to specify what
> > + action
> > + * should take place for the slave. Returns 0 if no changes are
> > + found, >0
> > + * if changes to link states must be committed.
> > *
> > * Called with rcu_read_lock held.
> > */
> > @@ -2777,12 +2774,12 @@ static int bond_ab_arp_inspect(struct bonding
> *bond)
> > int commit = 0;
> >
> > bond_for_each_slave_rcu(bond, slave, iter) {
> > - slave->new_link = BOND_LINK_NOCHANGE;
> > + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
> > last_rx = slave_last_rx(bond, slave);
> >
> > if (slave->link != BOND_LINK_UP) {
> > if (bond_time_in_interval(bond, last_rx, 1)) {
> > - slave->new_link = BOND_LINK_UP;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_UP);
> > commit++;
> > }
> > continue;
> > @@ -2810,7 +2807,7 @@ static int bond_ab_arp_inspect(struct bonding
> *bond)
> > if (!bond_is_active_slave(slave) &&
> > !rcu_access_pointer(bond->current_arp_slave) &&
> > !bond_time_in_interval(bond, last_rx, 3)) {
> > - slave->new_link = BOND_LINK_DOWN;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_DOWN);
> > commit++;
> > }
> >
> > @@ -2823,7 +2820,7 @@ static int bond_ab_arp_inspect(struct bonding
> *bond)
> > if (bond_is_active_slave(slave) &&
> > (!bond_time_in_interval(bond, trans_start, 2) ||
> > !bond_time_in_interval(bond, last_rx, 2))) {
> > - slave->new_link = BOND_LINK_DOWN;
> > + bond_propose_link_state(slave,
> > + BOND_LINK_DOWN);
> > commit++;
> > }
> > }
> > @@ -2843,7 +2840,7 @@ static void bond_ab_arp_commit(struct bonding
> *bond)
> > struct slave *slave;
> >
> > bond_for_each_slave(bond, slave, iter) {
> > - switch (slave->new_link) {
> > + switch (slave->link_new_state) {
> > case BOND_LINK_NOCHANGE:
> > continue;
> >
> > @@ -2893,8 +2890,9 @@ static void bond_ab_arp_commit(struct bonding
> *bond)
> > continue;
> >
> > default:
> > - slave_err(bond->dev, slave->dev, "impossible: new_link %d on
> slave\n",
> > - slave->new_link);
> > + slave_err(bond->dev, slave->dev,
> > + "impossible: link_new_state %d on slave\n",
> > + slave->link_new_state);
> > continue;
> > }
> >
> > @@ -3133,6 +3131,7 @@ static int bond_slave_netdev_event(unsigned long
> event,
> > * let link-monitoring (miimon) set it right when correct
> > * speeds/duplex are available.
> > */
> > +/*XXX*/slave_info(bond_dev, slave_dev, "EVENT %lu llu %lu\n", event,
> > +slave->last_link_up);
> > if (bond_update_speed_duplex(slave) &&
> > BOND_MODE(bond) == BOND_MODE_8023AD) {
> > if (slave->last_link_up) diff --git
> > a/include/net/bonding.h b/include/net/bonding.h index
> > f7fe45689142..d416af72404b 100644
> > --- a/include/net/bonding.h
> > +++ b/include/net/bonding.h
> > @@ -159,7 +159,6 @@ struct slave {
> > unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
> > s8 link; /* one of BOND_LINK_XXXX */
> > s8 link_new_state; /* one of BOND_LINK_XXXX */
> > - s8 new_link;
> > u8 backup:1, /* indicates backup slave. Value corresponds with
> > BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> > inactive:1, /* indicates inactive slave */ @@ -549,7
> > +548,7 @@ static inline void bond_propose_link_state(struct slave
> > *slave, int state)
> >
> > static inline void bond_commit_link_state(struct slave *slave, bool
> > notify) {
> > - if (slave->link == slave->link_new_state)
> > + if (slave->link_new_state == BOND_LINK_NOCHANGE)
> > return;
> >
> > slave->link = slave->link_new_state;
> >
> >
> > ---
> > -Jay Vosburgh, jay.vosburgh@canonical.com
>
>
>
> --
> Best Regards,
> Aleksei Zakharov
> System administrator
> Selectel - hosting for professionals
> tel: +7 (812) 677-80-36
>
> www.selectel.com
^ 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