* Re: [PATCH net-next 00/11] qed: Status block changes
From: David Miller @ 2017-06-01 16:17 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev
In-Reply-To: <20170601122911.22493-1-Yuval.Mintz@cavium.com>
From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Thu, 1 Jun 2017 15:29:00 +0300
> The device maintains a CAM mapping of the internal status blocks
> and the various PF/VF MSI-x vector mappings.
> During initialization, the driver reads the HW memory and constructs
> a shadow SW implementation which it would later use for manipulation
> of interrupts. E.g., when enabling VFs and setting their MSI-x tables.
>
> The driver currently has some very strict assumptions on the order the
> entries are placed in the CAM. Specifically, it assumes that all entries
> belonging to a PF would be consecutive and in-order in the CAM, and that
> the VF entries would then follow. But there's no actual HW constraint
> enforcing this assumption [although management firmware does set it
> accordingly to same assumption initially].
>
> Since the CAM is re-configurable, there are now SW flows employeed
> by other OSes that might cause the assumption to be invalid.
> Such flows allow the PF to forfeit some of it's available interrupts
> in favor of its VFs or vice versa.
> While those are not employeed today by qed, we want to relax the
> assumptions as much as we can -
> both to allow functionality after PDA as well as allowing future
> compatibility where the driver would be loaded after a newer one has
> 'dirtied' the CAM configuration.
>
> In addition to patches meant for the above relaxation, the series
> also contains various cleanups & refactoring for interrupt logic
> [most of which is !semantic].
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH][net-next] net: dsa: make function ksz_rcv static
From: David Miller @ 2017-06-01 16:12 UTC (permalink / raw)
To: colin.king
Cc: Woojung.Huh, UNGLinuxDriver, andrew, vivien.didelot, f.fainelli,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170601122242.14341-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 1 Jun 2017 13:22:42 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> function ksz_rcv can be made static as it does not need to be
> in global scope. Reformat arguments to make it checkpatch warning
> free too.
>
> Cleans up sparse warning: "symbol 'ksz_rcv' was not declared. Should
> it be static?"
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store
From: Jonathan Corbet @ 2017-06-01 16:11 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: Larry Finger, kvalo, linux-wireless, b43-dev, netdev,
linux-kernel
In-Reply-To: <592F6843.9000204@163.com>
On Thu, 01 Jun 2017 09:05:07 +0800
Jia-Ju Bai <baijiaju1990@163.com> wrote:
> I admit my patches are not well tested, and they may not well fix the bugs.
> I am looking forward to opinions and suggestions :)
May I politely suggest that sending out untested locking changes is a
dangerous thing to do? You really should not be changing the locking in a
piece of kernel code without understanding very well what the lock is
protecting and being able to say why your changes are safe. Without that,
the risk of introducing subtle bugs is very high.
It looks like you have written a useful tool that could help us to make
the kernel more robust. If you are interested in my suggestion, I would
recommend that you post the sleep-in-atomic scenarios that you are
finding, but refrain from "fixing" them in any case where you cannot offer
a strong explanation of why your fix is correct.
Thanks for working to find bugs in the kernel!
jon
^ permalink raw reply
* Re: [PATCH net] cxgb4: update latest firmware version supported
From: David Miller @ 2017-06-01 16:10 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan
In-Reply-To: <1496317290-26368-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Thu, 1 Jun 2017 17:11:30 +0530
> Change t4fw_version.h to update latest firmware version
> number to 1.16.45.0.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Or Gerlitz @ 2017-06-01 16:08 UTC (permalink / raw)
To: David Miller
Cc: Tom Herbert, Jiri Pirko, Or Gerlitz, Linux Netdev List,
Saeed Mahameed, Roi Dayan, Paul Blakey
In-Reply-To: <20170530.121800.676375959648055594.davem@davemloft.net>
On Tue, May 30, 2017 at 7:18 PM, David Miller <davem@davemloft.net> wrote:
> The top level merge commit explains everything, and since the patches
> are grouped together logically, even if you just look at the merge
> commit in the GIT tree, it is mentioned.
Hi Dave,
Do you want to see a repost here or you are going to process the
series I submitted from patchworks?
Or.
^ permalink raw reply
* Re: [PATCH 0/5] Add phylib support for MV88X3310 10G phy
From: David Miller @ 2017-06-01 16:07 UTC (permalink / raw)
To: linux; +Cc: andrew, f.fainelli, devicetree, mark.rutland, netdev, robh+dt
In-Reply-To: <20170601102327.GF27796@n2100.armlinux.org.uk>
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Thu, 1 Jun 2017 11:23:27 +0100
> This patch series adds support for the Marvell 88x3310 PHY found on
> the SolidRun Macchiatobin board.
Andrew has asked for some comment documentation additions to patch #5
so I'm expecting at least one respin of this series ;-)
^ permalink raw reply
* Re: [PATCH net-next] ppp: remove unnecessary bh disable in xmit path
From: David Miller @ 2017-06-01 16:01 UTC (permalink / raw)
To: gfree.wind; +Cc: paulus, linux-ppp, g.nault, netdev
In-Reply-To: <1496311119-59204-1-git-send-email-gfree.wind@vip.163.com>
From: gfree.wind@vip.163.com
Date: Thu, 1 Jun 2017 17:58:39 +0800
> From: Gao Feng <gfree.wind@vip.163.com>
>
> Since the commit 55454a565836 ("ppp: avoid dealock on recursive xmit"),
> the PPP xmit path is protected by wrapper functions which disable the
> bh already. So it is unnecessary to disable the bh again in the real
> xmit path.
>
> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 2/3] udp: avoid a cache miss on dequeue
From: Eric Dumazet @ 2017-06-01 15:58 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, Eric Dumazet
In-Reply-To: <1496313592.4872.14.camel@redhat.com>
On Thu, 2017-06-01 at 12:39 +0200, Paolo Abeni wrote:
> On Wed, 2017-05-31 at 10:00 -0700, Eric Dumazet wrote:
> > On Mon, 2017-05-29 at 17:27 +0200, Paolo Abeni wrote:
> > > Since UDP no more uses sk->destructor, we can clear completely
> > > the skb head state before enqueuing.
> >
> > ...
> >
> > > @@ -1739,6 +1740,9 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > > sk_mark_napi_id_once(sk, skb);
> > > }
> > >
> > > + /* drop all pending head states; dst, nf and sk are dropped by caller */
> > > + secpath_reset(skb);
> > > +
> >
> > I wonder if using skb_release_head_state() would be more appropriate ?
> >
> > Surely more descriptive and probably not more expensive since all
> > cache lines should be already hot at this point.
>
> Thank you for reviewing this.
>
> I would prefer not adding more code to the core, but I think we would
> need something new, like:
>
> skb_reset_head_state()
> {
> skb_dst_drop(skb);
> secpath_reset(skb);
> nf_reset(skb);
> skb_orphan(skb);
> }
>
> because elsewhere the skb could be in inconsistent state: skb->sp !=
> NULL but with its refcount is already decremented. WDYT?
I do not believe skb->sk is set anymore in UDP receive path.
If early demux sets skb->sk for a moment, skb_steal_sock() would set
skb->sk back to NULL
^ permalink raw reply
* Re: [PATCH net V3] net/mlx4: Fix the check in attaching steering rules
From: David Miller @ 2017-06-01 15:56 UTC (permalink / raw)
To: tariqt; +Cc: netdev, eranbe, ogerlitz, talatb
In-Reply-To: <1496308606-13653-1-git-send-email-tariqt@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Date: Thu, 1 Jun 2017 12:16:46 +0300
> From: Talat Batheesh <talatb@mellanox.com>
>
> Our previous patch (cited below) introduced a regression
> for RAW Eth QPs.
>
> Fix it by checking if the QP number provided by user-space
> exists, hence allowing steering rules to be added for valid
> QPs only.
>
> Fixes: 89c557687a32 ("net/mlx4_en: Avoid adding steering rules with ...")
Please do not truncate the commit header line text, always provide the
whole thing and do not break up the Fixes: line with newlines even if
the line exceeds 80 columns so people can grep it into the logs.
Thank you.
^ permalink raw reply
* Re: [net] vxlan: fix use-after-free on deletion
From: David Miller @ 2017-06-01 15:54 UTC (permalink / raw)
To: markb; +Cc: jbenc, roopa, netdev
In-Reply-To: <1496306615-770-1-git-send-email-markb@mellanox.com>
From: Mark Bloch <markb@mellanox.com>
Date: Thu, 1 Jun 2017 11:43:35 +0300
> Adding a vxlan interface to a socket isn't symmetrical, while adding
> is done in vxlan_open() the deletion is done in vxlan_dellink().
> This can cause a use-after-free error when we close the vxlan
> interface before deleting it.
>
> We add vxlan_vs_del_dev() to match vxlan_vs_add_dev() and call
> it from vxlan_stop() to match the call from vxlan_open().
>
> Signed-off-by: Mark Bloch <markb@mellanox.com>
Please add an appropriate Fixes: tag.
Thank you.
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Update module author
From: Thomas Falcon @ 2017-06-01 15:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, nfont, jallen
In-Reply-To: <20170531.171203.1567468797463479514.davem@davemloft.net>
On 05/31/2017 04:12 PM, David Miller wrote:
> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> Date: Wed, 31 May 2017 15:35:31 -0500
>
>> Update the module author to be one of the original co-maintainers
>> of the driver. The original author left the project long ago and
>> so far has not responded to emails sent to the listed address.
>>
>> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
>> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> It's MODULE_AUTHOR not MODULE_MAINTAINER.
>
Would it be better to include a second or more authors and/or remove the original author's mailing address, which isn't valid anymore? How should this be handled?
^ permalink raw reply
* Re: [net-next] team: add macro MODULE_ALIAS_TEAM_MODE for team mode alias
From: David Miller @ 2017-06-01 15:50 UTC (permalink / raw)
To: zhangshengju; +Cc: netdev, jiri
In-Reply-To: <1496302622-9958-1-git-send-email-zhangshengju@cmss.chinamobile.com>
From: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Date: Thu, 1 Jun 2017 15:37:02 +0800
> Add a new macro MODULE_ALIAS_TEAM_MODE to unify and simplify the
> declaration of team mode alias.
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Jiri, please review.
^ permalink raw reply
* Re: [PATCH net] netlink: don't send unknown nsid
From: David Miller @ 2017-06-01 15:50 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev, fbl
In-Reply-To: <1496304007-1962-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 1 Jun 2017 10:00:07 +0200
> The NETLINK_F_LISTEN_ALL_NSID otion enables to listen all netns that have a
> nsid assigned into the netns where the netlink socket is opened.
> The nsid is sent as metadata to userland, but the existence of this nsid is
> checked only for netns that are different from the socket netns. Thus, if
> no nsid is assigned to the socket netns, NETNSA_NSID_NOT_ASSIGNED is
> reported to the userland. This value is confusing and useless.
> After this patch, only valid nsid are sent to userland.
>
> Reported-by: Flavio Leitner <fbl@sysclose.org>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH 2/5] net: phy: hook up clause 45 autonegotiation restart
From: Russell King - ARM Linux @ 2017-06-01 15:47 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev
In-Reply-To: <20170601131955.GH9282@lunn.ch>
On Thu, Jun 01, 2017 at 03:19:55PM +0200, Andrew Lunn wrote:
> On Thu, Jun 01, 2017 at 02:09:00PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Jun 01, 2017 at 03:05:27PM +0200, Andrew Lunn wrote:
> > > So you are saying a 10G PHY driver always needs to have a aneg_done
> > > callback, even if it just needs to call phygen_c45_aneg_done?
> > >
> > > This seems a bit error prone. I can see somebody writing a 10G driver,
> > > leaving out aneg_done() and having the c22 version called. Is the read
> > > of MII_BMSR likely to return 0xffff, since the register does not
> > > exist? If so, genphy_aneg_done() is likely to always return
> > > BMSR_ANEGCOMPLETE.
> >
> > Don't forget that the read will fail, so phy_read() will return a
> > negative number.
>
> By fail, you mean return something like -EIO or -ETIMEOUT? Is this
> guaranteed in the code somewhere? This particular Marvell PHY only
> does c45. But i could imagine some other PHYs answering a c22 request
> with 0xffff.
Yes, C45 allows the PHYs to answer C22 as well, but then they have to
implement the C22 register set. Such a PHY would be out of spec,
especially as what you're suggesting is that it answers C22 cycles
and fails to implement MII_BMSR. I also think that there's a comment
in the 802.3 specs that says that unimplemented registers are to
return zero, not 0xffff.
So, if there is a non-compliant PHY like that, I think the time to
address it is when such a broken situation appears, rather than
engineering additional complexity before we know we need it.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH] qlcnic: Fix a sleep-in-atomic bug in qlcnic_82xx_hw_write_wx_2M and qlcnic_82xx_hw_read_wx_2M
From: David Miller @ 2017-06-01 15:42 UTC (permalink / raw)
To: baijiaju1990
Cc: harish.patil, manish.chopra, Dept-GELinuxNICDev, netdev,
linux-kernel
In-Reply-To: <1496305090-17814-1-git-send-email-baijiaju1990@163.com>
From: Jia-Ju Bai <baijiaju1990@163.com>
Date: Thu, 1 Jun 2017 16:18:10 +0800
> The driver may sleep under a write spin lock, and the function
> call path is:
> qlcnic_82xx_hw_write_wx_2M (acquire the lock by write_lock_irqsave)
> crb_win_lock
> qlcnic_pcie_sem_lock
> usleep_range
> qlcnic_82xx_hw_read_wx_2M (acquire the lock by write_lock_irqsave)
> crb_win_lock
> qlcnic_pcie_sem_lock
> usleep_range
>
> To fix it, the usleep_range is replaced with udelay.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
Applied.
^ permalink raw reply
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Jason Gunthorpe @ 2017-06-01 15:37 UTC (permalink / raw)
To: Ilan Tayari
Cc: Alexei Starovoitov, Saeed Mahameed, David S. Miller, Doug Ledford,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jsorensen-b10kYP2dOMg@public.gmane.org, Andy Shevchenko,
linux-fpga-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alan Tull,
yi1.li-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, Boris Pismenny
In-Reply-To: <AM4PR0501MB19409139227E11A4A7F82F0FDBF30-dp/nxUn679gfNUYDR5dMTsDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Mon, May 29, 2017 at 04:09:06PM +0000, Ilan Tayari wrote:
> > For IPSec, this is already in the kernel.
> > See this patchset:
> > http://www.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg162876.html
>
> Sorry, I pointed at the RFC by mistake.
>
> This is the relevant pull request:
> https://patchwork.ozlabs.org/patch/752707/
This is connecting ipsec to a netdev, while Innova seems to be a
network connected ipsec accelerator configured using IP packets.
Those two things don't seem to be the same.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err_skb) has broken ping
From: Cyril Hrubis @ 2017-06-01 15:36 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David S. Miller, Shmulik Ladkani, Marcelo Ricardo Leitner,
Pravin B Shelar, Eric Dumazet, WANG Cong, Yaogong Wang,
Steffen Klassert, Al Viro, netdev, linux-kernel
In-Reply-To: <CACSApvanT0ZRFH0ZBDLqygkH6Ue_CCVQBzVL9fCmZE_BndYEog@mail.gmail.com>
Hi!
> >> Thank you for the confirmation. Could you please try the following
> >> patch to see if it fixes your issue?
> >
> > Does not seem to help, I still got the same bussy loop.
>
> Thank you for trying the patch. Unfortunately, I can't reproduce on my
> machines here.Would you humor me with another one? Thank you!
Still does not help.
I've modified the ping binary (on the top of you patch that reads the
error queue) to dump the error structure.
It seems to repeatedly produce (until I plug the cable back):
ee_errno = 113 ee_origin = 2 ee_type = 3 ee_code = 1 ee_info = 0 ee_data = 0
So we get EHOSTUNREACH on SO_EE_ORIGIN_ICMP.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply
* Re: [PATCH net-next] ipv4: route: restore skb_dst_set in inet_rtm_getroute
From: David Miller @ 2017-06-01 15:34 UTC (permalink / raw)
To: roopa; +Cc: netdev, john.fastabend, dsahern
In-Reply-To: <1496296405-61386-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Wed, 31 May 2017 22:53:25 -0700
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> recent updates to inet_rtm_getroute dropped skb_dst_set in
> inet_rtm_getroute. This patch restores it because it is
> needed to release the dst correctly.
>
> Fixes: 3765d35ed8b9 ("net: ipv4: Convert inet_rtm_getroute to rcu versions of route lookup")
> Reported-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/3] perf, bpf: Add BPF support to all perf_event types
From: Alexei Starovoitov @ 2017-06-01 15:21 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David S . Miller, Brendan Gregg, Daniel Borkmann, Teng Qin,
netdev, linux-kernel
In-Reply-To: <20170601133216.oxroqy4z5qjwvrdm@hirez.programming.kicks-ass.net>
On 6/1/17 6:32 AM, Peter Zijlstra wrote:
> OK, do that. Something like the below should do I suppose.
>
> It will return -EOPNOTSUPP for permanent failure (it cannot ever work)
> and -EINVAL for temporary failure (could work if you call it on another
> task/cpu).
Thanks! Will test it and report back.
^ permalink raw reply
* Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err_skb) has broken ping
From: Soheil Hassas Yeganeh @ 2017-06-01 15:15 UTC (permalink / raw)
To: Cyril Hrubis
Cc: David S. Miller, Shmulik Ladkani, Marcelo Ricardo Leitner,
Pravin B Shelar, Eric Dumazet, WANG Cong, Yaogong Wang,
Steffen Klassert, Al Viro, netdev, linux-kernel
In-Reply-To: <20170601151003.GB10526@rei.suse.de>
On Thu, Jun 1, 2017 at 11:10 AM, Cyril Hrubis <chrubis@suse.cz> wrote:
>> Thank you for the confirmation. Could you please try the following
>> patch to see if it fixes your issue?
>
> Does not seem to help, I still got the same bussy loop.
Thank you for trying the patch. Unfortunately, I can't reproduce on my
machines here.Would you humor me with another one? Thank you!
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5a726161f4e4..49207298fcea 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3742,7 +3742,8 @@ EXPORT_SYMBOL(sock_queue_err_skb);
static bool is_icmp_err_skb(const struct sk_buff *skb)
{
return skb && (SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
- SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP6);
+ SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_ICMP6 ||
+ SKB_EXT_ERR(skb)->ee.ee_origin == SO_EE_ORIGIN_LOCAL);
}
struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
^ permalink raw reply related
* Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err_skb) has broken ping
From: Cyril Hrubis @ 2017-06-01 15:10 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David S. Miller, Shmulik Ladkani, Marcelo Ricardo Leitner,
Pravin B Shelar, Eric Dumazet, WANG Cong, Yaogong Wang,
Steffen Klassert, Al Viro, netdev, linux-kernel
In-Reply-To: <CACSApvbKwbX2u16V1oQfmHC7SvGZgk-KehA9tashjQAtj0bB0Q@mail.gmail.com>
Hi!
> Thank you for the confirmation. Could you please try the following
> patch to see if it fixes your issue?
Does not seem to help, I still got the same bussy loop.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply
* [PATCH net] net: bridge: start hello timer only if device is up
From: Nikolay Aleksandrov @ 2017-06-01 15:07 UTC (permalink / raw)
To: netdev
Cc: davem, roopa, Nikolay Aleksandrov, Xin Long, Ivan Vecera,
Sebastian Ott
In-Reply-To: <06ef2ff1-f29d-a163-3226-7bd43c7a407c@cumulusnetworks.com>
When the transition of NO_STP -> KERNEL_STP was fixed by always calling
mod_timer in br_stp_start, it introduced a new regression which causes
the timer to be armed even when the bridge is down, and since we stop
the timers in its ndo_stop() function, they never get disabled if the
device is destroyed before it's upped.
To reproduce:
$ while :; do ip l add br0 type bridge hello_time 100; brctl stp br0 on;
ip l del br0; done;
CC: Xin Long <lucien.xin@gmail.com>
CC: Ivan Vecera <cera@cera.cz>
CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
Reported-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Fixes: 6d18c732b95c ("bridge: start hello_timer when enabling KERNEL_STP in br_stp_start")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Sebastian it'd be great if you can test the patch as well.
net/bridge/br_stp_if.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 4efd5d54498a..89110319ef0f 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -173,7 +173,8 @@ static void br_stp_start(struct net_bridge *br)
br_debug(br, "using kernel STP\n");
/* To start timers on any ports left in blocking */
- mod_timer(&br->hello_timer, jiffies + br->hello_time);
+ if (br->dev->flags & IFF_UP)
+ mod_timer(&br->hello_timer, jiffies + br->hello_time);
br_port_state_selection(br);
}
--
2.1.4
^ permalink raw reply related
* [PATCH V7 net-next iproute] ip: Add IFLA_EVENT output to ip monitor
From: Vladislav Yasevich @ 2017-06-01 15:01 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, Vladislav Yasevich
Add IFLA_EVENT output so that event types can be viewed with
'monitor' command. This gives a little more information for why
a given message was received.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
v7: change the names to make it more generic then just netdev events.
Updated initialization.
include/linux/if_link.h | 11 +++++++++++
ip/ipaddress.c | 22 ++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 5a3a048..c0a6769 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
IFLA_GSO_MAX_SIZE,
IFLA_PAD,
IFLA_XDP,
+ IFLA_EVENT,
__IFLA_MAX
};
@@ -909,4 +910,14 @@ enum {
#define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
+enum {
+ IFLA_EVENT_NONE,
+ IFLA_EVENT_REBOOT, /* internal reset / reboot */
+ IFLA_EVENT_FEATURES, /* change in offload features */
+ IFLA_EVENT_BONDING_FAILOVER, /* hange in active slave */
+ IFLA_EVENT_NOTIFY_PEERS, /* re-sent grat. arp/ndisc */
+ IFLA_EVENT_IGMP_RESEND, /* re-sent IGMP JOIN */
+ IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */
+};
+
#endif /* _LINUX_IF_LINK_H */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b8d9c7d..cea4389 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -753,6 +753,25 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
return 0;
}
+static const char *link_events[] = {
+ [IFLA_EVENT_NONE] = "NONE",
+ [IFLA_EVENT_REBOOT] = "REBOOT",
+ [IFLA_EVENT_FEATURES] = "FEATURE CHANGE",
+ [IFLA_EVENT_BONDING_FAILOVER] = "BONDING FAILOVER",
+ [IFLA_EVENT_NOTIFY_PEERS] = "NOTIFY PEERS",
+ [IFLA_EVENT_IGMP_RESEND] = "RESEND IGMP",
+ [IFLA_EVENT_BONDING_OPTIONS] = "BONDING OPTION"};
+
+static void print_link_event(FILE *f, __u32 event)
+{
+ if (event >= ARRAY_SIZE(link_events))
+ fprintf(f, "event %d ", event);
+ else {
+ if (event)
+ fprintf(f, "event %s ", link_events[event]);
+ }
+}
+
int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
{
@@ -858,6 +877,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
if (filter.showqueue)
print_queuelen(fp, tb);
+ if (tb[IFLA_EVENT])
+ print_link_event(fp, rta_getattr_u32(tb[IFLA_EVENT]));
+
if (!filter.family || filter.family == AF_PACKET || show_details) {
SPRINT_BUF(b1);
fprintf(fp, "%s", _SL_);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument
From: Djalal Harouni @ 2017-06-01 14:56 UTC (permalink / raw)
To: Kees Cook
Cc: Serge E. Hallyn, Rusty Russell, David S . Miller, Jessica Yu,
LKML, Network Development, linux-security-module,
kernel-hardening@lists.openwall.com, Andy Lutomirski,
Andrew Morton, James Morris, Paul Moore, Stephen Smalley,
Greg Kroah-Hartman, Tetsuo Handa, Ingo Molnar, Linux API,
Dongsu Park, Casey Schaufler, Jonathan Corbet,
Arnaldo Carvalho de Melo
In-Reply-To: <CAGXu5jK5exWBV+ub8ys86Ubu1vKwN=miOZxwp3zdWaWEfuJddQ@mail.gmail.com>
On Tue, May 30, 2017 at 7:59 PM, Kees Cook <keescook@google.com> wrote:
[...]
>>> I see a few options:
>>>
>>> 1) keep what you have for v4, and hope other places don't use
>>> __request_module. (I'm not a fan of this.)
>>
>> Yes even if it is documented I wouldn't bet on it, though. :-)
>
> Okay, we seem to agree: we'll not use #1.
>
>>> 2) switch the logic on autoload==1 from OR to AND: both the specified
>>> caps _and_ CAP_SYS_MODULE are required. (This seems like it might make
>>> autoload==1 less useful.)
>>
>> That will restrict some userspace that works only with CAP_NET_ADMIN.
>
> Nor #2.
>
>>> 3) use the request_module_cap() outlined above, which requires that
>>> modules being loaded under a CAP_SYS_MODULE-aliased capability are at
>>> least restricted to a subset of kernel module names.
>>
>> This one tends to allow usability.
>
> Right, discussed below...
>
>>> 4) same as 3 but also insert autoload==2 level that switches from OR
>>> to AND (bumping existing ==2 to ==3).
>>
>> I wouldn't expose autoload to callers, I think it is better if it
>> stays a property of the module subsystem. But lets use the bump idea,
>> please see below.
>
> If we can't agree below, I think #4 would be a good way to allow for
> both states.
Ok!
>>> What do you think?
>>
>> Ok so given that we already have modules_autoload_mode=2 disabled,
>> maybe we go with 3) like this ?
>>
>> int __request_module(bool wait, int required_cap, const char *prefix,
>> const char *name, ...);
>> #define request_module(mod...) \
>> __request_module(true, -1, NULL, mod)
>> #define request_module_cap(required_cap, prefix, mod...) \
>> __request_module(true, required_cap, prefix, mod)
>>
>> and we require allow_cap and prefix to be set.
>>
>> request_module_cap(CAP_NET_ADMIN, "netdev-", "%s", name) for
>> net/core/dev_ioctl.c:dev_load()
>>
>> request_module_cap(CAP_NET_ADMIN, "tcp_", "%s", name) for
>> net/ipv4/tcp_cong.c functions.
>>
>>
>> Then
>> __request_module()
>> -> security_kernel_module_request(module_name, required_cap, prefix)
>> -> may_autoload_module(current, module_name, required_cap, prefix)
>>
>>
>> And update may_autoload_module() as below ? we hard code CAP_NET_ADMIN
>> and CAP_SYS_MODULE inside and make them the only capabilities needed
>> for a privileged auto-load operation.
>
> I still think making a specific exception for CAP_NET_ADMIN is not the
> right solution, instead allowing for non-CAP_SYS_MODULE caps when
> using a distinct prefix.
Alright! I would have loved to avoid capabilities game, but these
patches also use them... so worst scenario is the per-task can always
be set, "task->module_autoload_mode=2" and block it if necessary.
>> request_module_cap(CAP_SYS_MODULE, ...) or
>> request_module_cap(CAP_NET_ADMIN, ...) if the autoload should be a
>> privileged operation.
>>
>> Kees will this work ?
>>
>> Jessica, Rusty, Serge. What do you think ? I definitively think that
>> module_autoload should be contained only inside the module subsystem..
>
> I'd change it like this:
>
>> +int may_autoload_module(struct task_struct *task, char *kmod_name,
>> + int require_cap, char *prefix)
>> +{
>> + unsigned int autoload;
>> + int module_require_cap = 0;
>
> I'd initialize this to module_require_cap = CAP_SYS_MODULE;
Ok, please see below.
>> +
>> + if (require_cap > 0) {
>> + if (prefix == NULL || *prefix == '\0')
>> + return -EPERM;
>
> Since an unprefixed module load should only be CAP_SYS_MODULE, change
> the above "if" to:
>
> if (require_cap > 0 && prefix != NULL && *prefix != '\0')
>
>> +
>> + /*
>> + * We only allow CAP_SYS_MODULE or CAP_NET_ADMIN for
>> + * 'netdev-%s' modules for backward compatibility.
>> + * Please do not overload capabilities.
>> + */
>> + if (require_cap == CAP_SYS_MODULE ||
>> + require_cap == CAP_NET_ADMIN)
>> + module_require_cap = require_cap;
>> + else
>> + return -EPERM;
>> + }
>
> And then drop all these checks, leaving only:
>
> module_require_cap = require_cap;
>
>> +
>> + /* Get max value of sysctl and task "modules_autoload_mode" */
>> + autoload = max_t(unsigned int, modules_autoload_mode,
>> + task->modules_autoload_mode);
>> +
>> + /*
>> + * If autoload is disabled then fail here and not bother at all
>> + */
>> + if (autoload == MODULES_AUTOLOAD_DISABLED)
>> + return -EPERM;
>> +
>> + /*
>> + * If caller require capabilities then we may not allow
>> + * automatic module loading. We should not bypass callers.
>> + * This allows to support networking code that uses CAP_NET_ADMIN
>> + * for some aliased 'netdev-%s' modules.
>> + *
>> + * Explicitly bump autoload here if necessary
>> + */
>> + if (module_require_cap && autoload == MODULES_AUTOLOAD_ALLOWED)
>> + autoload = MODULES_AUTOLOAD_PRIVILEGED;
>
> I don't see a reason to bump the autoload level.
>
>> +
>> + if (autoload == MODULES_AUTOLOAD_ALLOWED)
>> + return 0;
>
> This test can be moved to above the AUTOLOAD_DISABLED test.
>
>> + else if(autoload == MODULES_AUTOLOAD_PRIVILEGED) {
>> + /*
>> + * If module auto-load is a privileged operation then check
>> + * if capabilities are set.
>> + */
>> + if (capable(CAP_SYS_MODULE) ||
>> + (module_require_cap && capable(module_require_cap)))
>> + return 0;
>> + }
>
> This test could drop the explicit CAP_SYS_MODULE test and just rely on
> module_require_cap.
>
>> +
>> + return -EPERM;
>> +}
>> +
>
> So, I would suggest:
Ok Kees, I will update based on your feedback, except for the
following, please see below and let me know :-)
>
> int may_autoload_module(struct task_struct *task, char *kmod_name,
> int require_cap, char *prefix)
> {
> unsigned int autoload;
> int module_require_cap;
>
> if (autoload == MODULES_AUTOLOAD_DISABLED)
> return -EPERM;
>
> /* Get max value of sysctl and task "modules_autoload_mode" */
> autoload = max_t(unsigned int, modules_autoload_mode,
> task->modules_autoload_mode);
>
> if (autoload == MODULES_AUTOLOAD_ALLOWED)
> return 0;
I don't think that the MODULES_AUTOLOAD_ALLOWED check here at this
place is the best thing to do.
If we remove the capable(CAP_NET_ADMIN) from net/core/dev_ioctl:dev_load()
http://elixir.free-electrons.com/linux/v4.12-rc3/source/net/core/dev_ioctl.c#L369
Or if future changes (accidental) remove that capable(CAP_NET_ADMIN)
and replace it with:
request_module_cap(CAP_NET_ADMIN, "netdev-", "%s", name);
Then we will check the requested capability *after* autoload allowed
as it is in this example, it should be checked *before* the autoload
allowed:
// Check required capability before this
if (autoload == MODULES_AUTOLOAD_ALLOWED)
return 0;
This way we are still safe we do not downgrade the required capability
that was requested by the calling subsystem based on
MODULES_AUTOLOAD_ALLOWED. If networking code or any other code thinks
that we need CAP_X to load a module then we should honor it. So we do
not break current usage by introducing the "modules_autoload_mode", it
should be set regardless of the autoload mode. Especially since
modules autoload mode is 0 by default. This avoids breaking current
rule to require CAP_NET_ADMIN for 'netdevè-%'
> /*
> * It should be impossible for autoload to have any other
> * value at this point, so explicitly reject all other states.
> */
> if (autoload != MODULES_AUTOLOAD_PRIVILEGED)
> return -EPERM;
>
> /* Verify that alternate capabilities requirements had a prefix. */
> if (require_cap > 0 && prefix != NULL && *prefix != '\0')
> module_require_cap = require_cap;
> else
> module_require_cap = CAP_SYS_MODULE;
>
> return capable(module_require_cap);
So with your code, but I really think that we should treat
MODULES_AUTOLOAD_ALLOWED with special care in regard of the passed
capabilities, so:
module_require_cap = 0;
if (autoload == MODULES_AUTOLOAD_DISABLED)
return -EPERM;
if (autoload == MODULES_AUTOLOAD_PRIVILEGED || require_cap > 0) {
if (prefix != NULL && *prefix != '\0')
/*
* Allow non-CAP_SYS_MODULE caps when
* using a distinct prefix.
*/
module_require_cap = require_cap;
else
/*
* Otherwise always require CAP_SYS_MODULE if no
* valid prefix. Callers that do not provide a
valid prefix
* should not provide a require_cap > 0
*/
module_require_cap = CAP_SYS_MODULE;
}
/* If autoload allowed and 'module_require_cap' was *never*
set, allow */
if (module_require_cap == 0 && autoload == MODULES_AUTOLOAD_ALLOWED)
return 0;
return capable(module_require_cap) ? 0 : -EPERM;
> }
>
Maybe you will agree :-) ?
BTW Kees, also in next version I won't remove the
capable(CAP_NET_ADMIN) check from [1]
even if there is the new request_module_cap(), I would like it to be
in a different patches, this way we go incremental
and maybe it is better to merge what we have now ? and follow up
later, and of course if other maintainers agree too!
I just need a bit of free time to check again everything and will send
a v5 with all requested changes.
Thank you Kees for your time!
[1] http://elixir.free-electrons.com/linux/v4.12-rc3/source/net/core/dev_ioctl.c#L369
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Timur Tabi @ 2017-06-01 14:48 UTC (permalink / raw)
To: Zefir Kurtisi, Matthias May, Andrew Lunn
Cc: netdev, f.fainelli, David Miller, Manoj Iyer, jhugo
In-Reply-To: <265ee4d4-ce24-9e0f-d7c5-fba4ed36ab96@neratec.com>
On 06/01/2017 06:45 AM, Zefir Kurtisi wrote:
> I guess we need to decide whether we generally need to handle permanent aneg
> failures on the SGMII link. If we expect that it must not fail (like we assumed
> until we saw it failing), I agree with Timur and support reverting of the related
> commit f62265b53e. If otherwise we want this potential failure to be handled
> correctly, things become arbitrary complex. Essentially, we need to handle such
> PHYs as a combination of their two sides (copper + SGMII) as virtual sub-PHYs. The
> phylib might support that in a future version, but for now this seems like a lot
> of work required to handle a rare problem.
I'm about to post a patch that removes interrupt support from the EMAC
driver and relies on software polling of the PHY. With this patch, we
don't see the "link is not okay" message from that at803x driver any more.
The link state is generally more reliable now, even when the at803x
driver doesn't complain.
My theory is that the hardware polling of the PHY is just too
aggressive. I think it continuously reads the PHY status register at
maximum speed and immediately issues an interrupt when the PHY says that
it's up.
So I think we're okay with leaving the at803x driver as-is, since we
appear to be no longer getting any false failures.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ 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