* Re: [pull request][for-next] Mellanox mlx5 Reorganize core driver directory layout
From: David Miller @ 2017-01-19 5:38 UTC (permalink / raw)
To: tom; +Cc: saeedm, saeedm, dledford, netdev, linux-rdma, leon
In-Reply-To: <CALx6S36p2g8EP48R+nWQu9tHtS5h8eKo7bj7rskmSMxCU36KXA@mail.gmail.com>
From: Tom Herbert <tom@herbertland.com>
Date: Wed, 18 Jan 2017 21:22:53 -0800
> In principle I agree, but as I previously wrote backporting this
> driver is already a bear. We need to do something here as this is
> quickly approaching being infeasible to backport. Even if we don't
> restructure the directories I'd still like to see some effort towards
> feature modularization and isolation.
There simply is a lot changing all the time, no restructuring is going
to reduce that. The driver simply gets a lot of churn.
Backporting such things through a bunch of file and/or directory
renames is seriously nothing but asking for more punishment.
That's why I am against it.
^ permalink raw reply
* Re: [PATCH] stmmac: adding EEE to GMAC4
From: Rayagond Kokatanur @ 2017-01-19 6:01 UTC (permalink / raw)
To: Joao Pinto
Cc: David Miller, Giuseppe Cavallaro, Alexandre Torgue, linux-kernel,
netdev
In-Reply-To: <f89aaac8024e2a92834088545ad72fbe682b32ed.1483030665.git.jpinto@synopsys.com>
On Thu, Dec 29, 2016 at 10:40 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
> This patch adds Energy Efficiency Ethernet to GMAC4.
>
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 12 +++++
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 59 +++++++++++++++++++++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index b524598..73d1dab 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -90,6 +90,18 @@ enum power_event {
> power_down = 0x00000001,
> };
>
> +/* Energy Efficient Ethernet (EEE) for GMAC4
> + *
> + * LPI status, timer and control register offset
> + */
> +#define GMAC4_LPI_CTRL_STATUS 0xd0
> +#define GMAC4_LPI_TIMER_CTRL 0xd4
> +
> +/* LPI control and status defines */
> +#define GMAC4_LPI_CTRL_STATUS_LPITXA BIT(19) /* Enable LPI TX Automate */
> +#define GMAC4_LPI_CTRL_STATUS_PLS BIT(17) /* PHY Link Status */
> +#define GMAC4_LPI_CTRL_STATUS_LPIEN BIT(16) /* LPI Enable */
> +
> /* MAC Debug bitmap */
> #define GMAC_DEBUG_TFCSTS_MASK GENMASK(18, 17)
> #define GMAC_DEBUG_TFCSTS_SHIFT 17
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> index ecfbf57..02eab79 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> @@ -137,6 +137,61 @@ static void dwmac4_get_umac_addr(struct mac_device_info *hw,
> GMAC_ADDR_LOW(reg_n));
> }
>
> +static void dwmac4_set_eee_mode(struct mac_device_info *hw)
> +{
> + void __iomem *ioaddr = hw->pcsr;
> + u32 value;
> +
> + /* Enable the link status receive on RGMII, SGMII ore SMII
> + * receive path and instruct the transmit to enter in LPI
> + * state.
> + */
> + value = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
> + value |= GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA;
> +
> + writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
> +}
> +
> +static void dwmac4_reset_eee_mode(struct mac_device_info *hw)
> +{
> + void __iomem *ioaddr = hw->pcsr;
> + u32 value;
> +
> + value = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
> + value &= ~(GMAC4_LPI_CTRL_STATUS_LPIEN | GMAC4_LPI_CTRL_STATUS_LPITXA);
> + writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
> +}
> +
> +static void dwmac4_set_eee_pls(struct mac_device_info *hw, int link)
> +{
> + void __iomem *ioaddr = hw->pcsr;
> + u32 value;
> +
> + value = readl(ioaddr + GMAC4_LPI_CTRL_STATUS);
> +
> + if (link)
> + value |= GMAC4_LPI_CTRL_STATUS_PLS;
> + else
> + value &= ~GMAC4_LPI_CTRL_STATUS_PLS;
> +
> + writel(value, ioaddr + GMAC4_LPI_CTRL_STATUS);
> +}
> +
> +static void dwmac4_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
> +{
> + void __iomem *ioaddr = hw->pcsr;
> + int value = ((tw & 0xffff)) | ((ls & 0x7ff) << 16);
If I am not wrong, LS field is 10 bits wide not 11 bits, hence we
should AND 0x3ff.
int value = ((tw & 0xffff)) | ((ls & 0x3ff) << 16);
> +
> + /* Program the timers in the LPI timer control register:
> + * LS: minimum time (ms) for which the link
> + * status from PHY should be ok before transmitting
> + * the LPI pattern.
> + * TW: minimum time (us) for which the core waits
> + * after it has stopped transmitting the LPI pattern.
> + */
> + writel(value, ioaddr + GMAC4_LPI_TIMER_CTRL);
> +}
> +
> static void dwmac4_set_filter(struct mac_device_info *hw,
> struct net_device *dev)
> {
> @@ -410,6 +465,10 @@ static const struct stmmac_ops dwmac4_ops = {
> .pmt = dwmac4_pmt,
> .set_umac_addr = dwmac4_set_umac_addr,
> .get_umac_addr = dwmac4_get_umac_addr,
> + .set_eee_mode = dwmac4_set_eee_mode,
> + .reset_eee_mode = dwmac4_reset_eee_mode,
> + .set_eee_timer = dwmac4_set_eee_timer,
> + .set_eee_pls = dwmac4_set_eee_pls,
> .pcs_ctrl_ane = dwmac4_ctrl_ane,
> .pcs_rane = dwmac4_rane,
> .pcs_get_adv_lp = dwmac4_get_adv_lp,
> --
> 2.9.3
>
--
wwr
Rayagond
^ permalink raw reply
* Re: [RFC] RESEND - rdmatool - tool for RDMA users
From: Leon Romanovsky @ 2017-01-19 6:04 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Ariel Almog, linux-rdma@vger.kernel.org, Linux Netdev List
In-Reply-To: <CAJ3xEMiR1+NGghUZJ6aGq+=xTdOHU5Ph-BcPii5OUB8dT4Vq-A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
On Wed, Jan 18, 2017 at 07:50:26PM +0200, Or Gerlitz wrote:
> On Wed, Jan 18, 2017 at 7:33 PM, Leon Romanovsky wrote:
> > On Wed, Jan 18, 2017 at 06:48:21PM +0200, Or Gerlitz wrote:
> >> On Wed, Jan 18, 2017 at 5:19 PM, Ariel Almog
> >> <arielalmogworkemails@gmail.com> wrote:
>
> >> What's wrong with the RDMA netlink infrastructure?! it's there for
> >> years and used
> >> for various cases by MLNX, did you look on the code?
>
> > Nothing wrong, it is one of the valuable options and will be used if
> > community decides to put this tool under iproute2/ethtool umbrella.
>
> yeah, using netlink sounds good to me, and where you package/maintain
> the tool is of 2nd order, 1st decide what UAPI you wonna use. So far
> netlink was good for what bunch of use-cases needed.
Or,
This tool is for users and they don't care how it is implemented
underneath. We want to provide to our users similar experience as they
have already with other tools and package/maintainability is important
for them more than kernel UAPI.
Ariel came with a small set of initial functionalities, but it doesn't
say we should limit ourselves.
>
> Or.
>
> >> cea05ea IB/core: Add flow control to the portmapper netlink calls
> >> ae43f82 IB/core: Add IP to GID netlink offload
> >> 2ca546b IB/sa: Route SA pathrecord query through netlink
> >> 753f618 RDMA/cma: Add support for netlink statistics export
> >> b2cbae2 RDMA: Add netlink infrastructure
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net] MAINTAINERS: update cxgb4 maintainer
From: Hariprasad Shenai @ 2017-01-19 6:20 UTC (permalink / raw)
To: netdev; +Cc: davem, nirranjan, ganeshgr, Hariprasad Shenai
Ganesg will be taking over as maintainer from now
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index c36976d3bd1a..af3456d33a18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3564,7 +3564,7 @@ F: drivers/infiniband/hw/cxgb3/
F: include/uapi/rdma/cxgb3-abi.h
CXGB4 ETHERNET DRIVER (CXGB4)
-M: Hariprasad S <hariprasad@chelsio.com>
+M: Ganesh Goudar <ganeshgr@chelsio.com>
L: netdev@vger.kernel.org
W: http://www.chelsio.com
S: Supported
--
2.3.4
^ permalink raw reply related
* Re: [pull request][for-next] Mellanox mlx5 Reorganize core driver directory layout
From: Doug Ledford @ 2017-01-19 6:32 UTC (permalink / raw)
To: David Miller, tom; +Cc: saeedm, saeedm, netdev, linux-rdma, leon
In-Reply-To: <20170119.003832.1262212358977773325.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 2808 bytes --]
On Thu, 2017-01-19 at 00:38 -0500, David Miller wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Wed, 18 Jan 2017 21:22:53 -0800
>
> > In principle I agree, but as I previously wrote backporting this
> > driver is already a bear. We need to do something here as this is
> > quickly approaching being infeasible to backport. Even if we don't
> > restructure the directories I'd still like to see some effort
> towards
> > feature modularization and isolation.
>
> There simply is a lot changing all the time, no restructuring is
> going
> to reduce that. The driver simply gets a lot of churn.
>
> Backporting such things through a bunch of file and/or directory
> renames is seriously nothing but asking for more punishment.
>
> That's why I am against it.
So, I spent the better part of two years dealing with this after the
drivers/net/mellanox drivers/net/ethernet/mellanox rename, so I'll
chime in. Especially since Tom's problem directly relates to the
issues I dealt with when trying to take the latest Mellanox driver and
shoe-horn it into a Red Hat kernel that didn't have the same net core
(I had to pick and choose which mlx patches to take in order to work
with our existing net core, and the files were renamed on top of that).
If you rename a file from place A to place B and keep the file the
same, then a git backport can be done using techniques such as git
cherry-pick with the --follow option enabled. It's slow, but works.
But that's not what you've brought up. You brought up continuing the
rename to go further and split code out into files based upon features.
While git cherry-pick can follow renames mostly automatically, it can
not follow refactors. If they move the tcflower stuff out to a file on
its own, and you then attempt to backport some fix that touches the
tcflower support in the new location while you have not taken the move
patch, you will find that git will be totally stymied.
What you have now is not perfect and you have to sort through commits
to get the right ones and skip the ones you can't support, but things
mostly apply except when context diffs break issues. With the refactor
and separation you speak of, git will be totally unable to help and you
will have to manually edit patches to point at new files or hand edit
the files to apply the patches.
I understand the desire for order instead of chaos, but it really will
not help. Bringing patches from the new order back to the old chaos
will be so much harder than just continuing as things are that you will
silently curse the change shortly after it is made.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC] RESEND - rdmatool - tool for RDMA users
From: Leon Romanovsky @ 2017-01-19 6:33 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jason Gunthorpe, Or Gerlitz, Ariel Almog,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Netdev List
In-Reply-To: <5f90fd26-e7bf-bb2a-01f2-6b166f2265e9-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]
On Wed, Jan 18, 2017 at 01:45:14PM -0800, Bart Van Assche wrote:
> On 01/18/2017 10:31 AM, Jason Gunthorpe wrote:
> > I think it depends on what this tool is supposed to cover, but based
> > on the description, I would start with netlink-only.
> >
> > The only place verbs covers a similar ground is in 'device
> > capabilities' - for some of that you might want to open a new-uAPI
> > verbs fd, but even the capability data from that would not be
> > totally offensive to be accessed over netlink.
> >
> > IMHO netlink should cover almost everything found in sysfs today.
>
> We would need a very strong argument to introduce a netlink API that
> duplicates existing sysfs API functionality. Since the sysfs API is
> extensible, why not extend that API further? E.g. the SCST sysfs API
> shows that more is possible with sysfs than what most kernel drivers
> realize.
We didn't look deeply on sysfs mainly because it is unpopular
in netdev community. Maybe we were misled and it is simply not true.
>
> Bart.
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH for bnxt_re V4 10/21] RDMA/bnxt_re: Support for CQ verbs
From: Doug Ledford @ 2017-01-19 6:57 UTC (permalink / raw)
To: Selvin Xavier, Leon Romanovsky
Cc: linux-rdma, Linux Netdev List, Michael Chan, Eddie Wai,
Devesh Sharma, Somnath Kotur, Sriharsha Basavapatna
In-Reply-To: <CA+sbYW0+Z2X+_BqG=QNBSgc0Qsk1S9sZYieUuN05PjF5or4NkQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 860 bytes --]
On Wed, 2017-01-18 at 14:42 +0530, Selvin Xavier wrote:
> On Wed, Jan 18, 2017 at 1:49 PM, Leon Romanovsky <leon@kernel.org>
> wrote:
> >
> > It is better to use already existing macros - min()
> > cq->max_cql = min(entries, MAX_CQL_PER_POLL);
> >
> > I afraid that you can't avoid the respinning, you have more than
> > month
> > till merge window.
> >
> > Can you please remove useless wrappers and try to reuse kernel
> > macros?
>
> Leon,
> Thanks for your comments. I will include your both comments in the
> next patch set.
>
> Doug,
> Is it ok if I send out the V5 now after re-basing against for-4.11
> branch or should i wait for your comments on V4?
Wait.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net] net/mlx5e: Do not recycle pages from emergency reserve
From: Eric Dumazet @ 2017-01-19 7:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed
From: Eric Dumazet <edumazet@google.com>
A driver using dev_alloc_page() must not reuse a page allocated from
emergency memory reserve.
Otherwise all packets using this page will be immediately dropped,
unless for very specific sockets having SOCK_MEMALLOC bit set.
This issue might be hard to debug, because only a fraction of received
packets would be dropped.
Fixes: 4415a0319f92 ("net/mlx5e: Implement RX mapped page cache for page recycle")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 0e2fb3ed1790900ccdc0bbbef8bc5c33267df092..ce46409bde2736ea4d1246ca97855098f052f271 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -193,6 +193,9 @@ static inline bool mlx5e_rx_cache_put(struct mlx5e_rq *rq,
return false;
}
+ if (unlikely(page_is_pfmemalloc(dma_info->page)))
+ return false;
+
cache->page_cache[cache->tail] = *dma_info;
cache->tail = tail_next;
return true;
^ permalink raw reply related
* please get back to me
From: Quatif Group of Companies. @ 2017-01-19 6:19 UTC (permalink / raw)
--
Dear Friend,
I would like to discuss a very important issue with you. I am writing to
find out if this is your valid email. Please, let me know if this email is
valid
Kind regards
Adrien Saif
Attorney to Quatif Group of Companies
^ permalink raw reply
* Re: [PATCH] ip/xfrm: Fix deleteall when having many policies installed
From: Alexander Heinlein @ 2017-01-19 7:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, shemminger
In-Reply-To: <20170118090302.6efd6cce@xeon-e3>
[-- Attachment #1.1: Type: text/plain, Size: 1069 bytes --]
From 192cf19b3a97871a508ad57ba5893d1719877f13 Mon Sep 17 00:00:00 2001
From: Alexander Heinlein <alexander.heinlein@secunet.com>
Date: Mon, 16 Jan 2017 14:48:25 +0100
Subject: [PATCH] ip/xfrm: Fix deleteall when having many policies installed
Fix "Policy buffer overflow" when trying to use deleteall with many
policies installed.
Signed-off-by: Alexander Heinlein <alexander.heinlein@secunet.com>
---
ip/xfrm_policy.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index cc9c0f1..451b982 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -732,10 +732,8 @@ static int xfrm_policy_keep(const struct sockaddr_nl *who,
if (!xfrm_policy_filter_match(xpinfo, ptype))
return 0;
- if (xb->offset > xb->size) {
- fprintf(stderr, "Policy buffer overflow\n");
- return -1;
- }
+ if (xb->offset + NLMSG_LENGTH(sizeof(*xpid)) > xb->size)
+ return 0;
new_n = (struct nlmsghdr *)(xb->buf + xb->offset);
new_n->nlmsg_len = NLMSG_LENGTH(sizeof(*xpid));
--
2.1.4
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* RE: [PATCH] net: fec: Fixed panic problem with non-tso
From: Andy Duan @ 2017-01-19 8:18 UTC (permalink / raw)
To: Eric Dumazet, Ashizuka, Yuusuke; +Cc: netdev@vger.kernel.org, Pravin B Shelar
In-Reply-To: <1484714132.13165.92.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com> Sent: Wednesday, January 18, 2017 12:36 PM
>To: Ashizuka, Yuusuke <ashiduka@jp.fujitsu.com>
>Cc: Andy Duan <fugang.duan@nxp.com>; netdev@vger.kernel.org; Pravin B
>Shelar <pshelar@nicira.com>
>Subject: Re: [PATCH] net: fec: Fixed panic problem with non-tso
>
>On Tue, 2017-01-17 at 20:21 -0800, Eric Dumazet wrote:
>> On Wed, 2017-01-18 at 03:12 +0000, Ashizuka, Yuusuke wrote:
>>
>> > indeed.
>> >
>> > In the case of TSO with i.MX6 system (highmem enabled) with 2GB
>> > memory, "this_frag->page.p" did not become highmem area.
>> > (We confirmed by transferring about 100MB of files)
>> >
>> > However, in the case of non-tso on an i.MX6 system with 2GB of
>> > memory, "this_frag->page.p" may become a highmem area.
>> > (Occurred with approximately 2MB of file transfer)
>> >
>> > For non-tso only, I do not know the reason why "this_frag-> page.p"
>> > in this driver shows highmem area.
>>
>> This worries me, since this driver does not set NETIF_F_HIGHDMA in its
>> features.
>>
>> No packet should be given to this driver with a highmem fragment
>>
>> Check is done in illegal_highdma() in net/core/dev.c
>
>This used to work.
>
>I suspect commit ec5f061564238892005257c83565a0b58ec79295
>("net: Kill link between CSUM and SG features.")
>
>added this bug.
>
>Can you try this hot fix :
>
>diff --git a/net/core/dev.c b/net/core/dev.c index
>ad5959e561166f445bdd9d7260652a338f74cfea..073b832b945257dba9ed47f4bf
>875605225effc9 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -2773,9 +2773,9 @@ static netdev_features_t harmonize_features(struct
>sk_buff *skb,
> if (skb->ip_summed != CHECKSUM_NONE &&
> !can_checksum_protocol(features, type)) {
> features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
>- } else if (illegal_highdma(skb->dev, skb)) {
>- features &= ~NETIF_F_SG;
> }
>+ if (illegal_highdma(skb->dev, skb))
>+ features &= ~NETIF_F_SG;
>
> return features;
> }
>
>
I will double check your fix. Thanks.
And, if driver is to support highmem, then we should add tso highmem support in net/core/tso.c, do you think it is necessary ?
^ permalink raw reply
* [PATCH] ipv6: addrconf: Avoid addrconf_disable_change() using RCU read-side lock
From: Kefeng Wang @ 2017-01-19 8:26 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
Cc: linux-kernel, dingtianhong, weiyongjun1, guohanjun, Kefeng Wang
Just like commit 4acd4945cd1e ("ipv6: addrconf: Avoid calling
netdevice notifiers with RCU read-side lock"), it is unnecessary
to make addrconf_disable_change() use RCU iteration over the
netdev list, since it already holds the RTNL lock, or we may meet
Illegal context switch in RCU read-side critical section.
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
net/ipv6/addrconf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c1e124b..f60e88e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5540,8 +5540,7 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
struct net_device *dev;
struct inet6_dev *idev;
- rcu_read_lock();
- for_each_netdev_rcu(net, dev) {
+ for_each_netdev(net, dev) {
idev = __in6_dev_get(dev);
if (idev) {
int changed = (!idev->cnf.disable_ipv6) ^ (!newf);
@@ -5550,7 +5549,6 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
dev_disable_change(idev);
}
}
- rcu_read_unlock();
}
static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int newf)
--
1.7.12.4
^ permalink raw reply related
* Re: linux-next: build warning after merge of the net tree
From: Tariq Toukan @ 2017-01-19 8:26 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking
Cc: linux-next, linux-kernel, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20170119095600.6d7a84d7@canb.auug.org.au>
Hi Stephen,
On 19/01/2017 12:56 AM, Stephen Rothwell wrote:
> Hi all,
>
> After merging the net tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c: In function 'mlx5e_set_channels':
> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:546:6: warning: unused variable 'ncv' [-Wunused-variable]
> int ncv = mlx5e_get_max_num_channels(priv->mdev);
> ^
>
> Introduced by commit
>
> 639e9e94160e ("net/mlx5e: Remove unnecessary checks when setting num channels")
>
> which removed the last reference to 'ncv'.
>
We will send a fix shortly.
Thanks for your report!
Regards,
Tariq
^ permalink raw reply
* Re: [PATCH net-next 0/9] mlx4 misc improvements
From: Tariq Toukan @ 2017-01-19 8:32 UTC (permalink / raw)
To: David Miller, tariqt; +Cc: netdev, eranbe
In-Reply-To: <20170117.144952.801420842795315435.davem@davemloft.net>
On 17/01/2017 9:49 PM, David Miller wrote:
> From: Tariq Toukan <tariqt@mellanox.com>
> Date: Mon, 16 Jan 2017 19:29:56 +0200
>
>> This patchset contains several improvements and cleanups
>> from the team to the mlx4 Eth and core drivers.
>>
>> Series generated against net-next commit:
>> dbeaa8c2a4ba stmmac: indent an if statement
> It looks like, at a minimum, we'll need a respin of this series to add
> some verbiage or explanations to the various commit log messages.
I'll consider the comments and submit a respin.
Thanks,
Tariq
^ permalink raw reply
* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Michal Hocko @ 2017-01-19 9:00 UTC (permalink / raw)
To: Tejun Heo
Cc: Peter Zijlstra, Andy Lutomirski, David Ahern, Alexei Starovoitov,
Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
Linux API, linux-kernel@vger.kernel.org, Network Development
In-Reply-To: <20170118221850.GF9171@mtj.duckdns.org>
On Wed 18-01-17 14:18:50, Tejun Heo wrote:
> Hello, Michal.
>
> On Tue, Jan 17, 2017 at 02:58:30PM +0100, Michal Hocko wrote:
> > This would require using hierarchical cgroup iterators to iterate over
>
> It does behave hierarchically.
>
> > tasks. As per Andy's testing this doesn't seem to be the case. I haven't
>
> That's not what Andy's testing showed. What that showed was that
> program in a child can override the one from its ancestor.
My fault, I've misread Andy's test case. I thought that the child group
simply disabled the bpf program and the one from the parent hasn't
executed.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH] net: ethernet: aquantia: fix alloc_cast.cocci warnings
From: kbuild test robot @ 2017-01-19 9:09 UTC (permalink / raw)
To: Alexander Loktionov
Cc: kbuild-all, netdev, David VomLehn, David S . Miller,
Simon Edelhaus, Alexander Loktionov, Dmitrii Tarakanov,
Pavel Belous, Dmitry Bezrukov
In-Reply-To: <bfce3d39db5d3cf8cac5e1cf355ffe13c2a08b4c.1484788010.git.vomlehn@texas.net>
drivers/net/ethernet/aquantia/atlantic/aq_ring.c:24:20-41: WARNING: casting value returned by memory allocation function to (struct aq_ring_buff_s *) is useless.
Remove casting the values returned by memory allocation functions
like kmalloc, kzalloc, kmem_cache_alloc, kmem_cache_zalloc etc.
Semantic patch information:
This makes an effort to find cases of casting of values returned by
kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
the casting as it is not required. The result in the patch case may
need some reformatting.
Generated by: scripts/coccinelle/api/alloc/alloc_cast.cocci
CC: David VomLehn <vomlehn@texas.net>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
aq_ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -21,7 +21,7 @@ static struct aq_ring_s *aq_ring_alloc(s
{
int err = 0;
- self->buff_ring = (struct aq_ring_buff_s *)
+ self->buff_ring =
kzalloc(sizeof(struct aq_ring_buff_s) * self->size, GFP_KERNEL);
if (!self->buff_ring) {
^ permalink raw reply
* Re: [PATCH v6 13/13] net: ethernet: aquantia: Integrate AQtion 2.5/5 GB NIC driver
From: kbuild test robot @ 2017-01-19 9:09 UTC (permalink / raw)
To: Alexander Loktionov
Cc: kbuild-all, netdev, David VomLehn, David S . Miller,
Simon Edelhaus, Alexander Loktionov, Dmitrii Tarakanov,
Pavel Belous, Dmitry Bezrukov
In-Reply-To: <bfce3d39db5d3cf8cac5e1cf355ffe13c2a08b4c.1484788010.git.vomlehn@texas.net>
Hi David,
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.10-rc4 next-20170118]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alexander-Loktionov/net-ethernet-aquantia-Make-and-configuration-files/20170119-133339
coccinelle warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/aquantia/atlantic/aq_ring.c:24:20-41: WARNING: casting value returned by memory allocation function to (struct aq_ring_buff_s *) is useless.
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH] net/mlx5e: Remove unused variable
From: Arnd Bergmann @ 2017-01-19 9:33 UTC (permalink / raw)
To: Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: Arnd Bergmann, David S. Miller, Gal Pressman, Tariq Toukan,
Maor Gottlieb, Eran Ben Elisha, netdev, linux-rdma, linux-kernel
A cleanup removed the only user of this variable
mlx5/core/en_ethtool.c: In function 'mlx5e_set_channels':
mlx5/core/en_ethtool.c:546:6: error: unused variable 'ncv' [-Werror=unused-variable]
Let's remove the declaration as well.
Fixes: 639e9e94160e ("net/mlx5e: Remove unnecessary checks when setting num channels")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index b1b9eb6ee135..5197817e4b2f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -543,7 +543,6 @@ static int mlx5e_set_channels(struct net_device *dev,
struct ethtool_channels *ch)
{
struct mlx5e_priv *priv = netdev_priv(dev);
- int ncv = mlx5e_get_max_num_channels(priv->mdev);
unsigned int count = ch->combined_count;
bool arfs_enabled;
bool was_opened;
--
2.9.0
^ permalink raw reply related
* Re: xennet_start_xmit assumptions
From: Paul Durrant @ 2017-01-19 9:36 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Sowmini Varadhan, Wei Liu
Cc: netdev@vger.kernel.org, xen-devel@lists.xenproject.org
In-Reply-To: <20170118192528.GA6847@char.us.oracle.com>
> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: 18 January 2017 19:25
> To: Sowmini Varadhan <sowmini.varadhan@oracle.com>; Wei Liu
> <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
> Cc: netdev@vger.kernel.org; xen-devel@lists.xenproject.org
> Subject: Re: [Xen-devel] xennet_start_xmit assumptions
>
> On Wed, Jan 18, 2017 at 10:31:32AM -0500, Sowmini Varadhan wrote:
> > As I was playing around with pf_packet, I accidentally wrote
> > a buggy application program that bzero'ed the msghdr, then set
> > up the msg_name, msg_namelen correctly, and then did a sendmsg
> > on the pf_packet/SOCK_RAW fd.
> >
> > This causes packet_snd to set up an skb with a lot of issues,
> > e.g., skb->len = 0, skb_headlen(skb) is 0, etc. I think we can/should
> > drop the packet in packet_snd if the skb->len is 0, but there
> > may be other driver bugs going on:
> >
> > Turns out that ixgbe and sunvnet handle this problematic
> > skb correctly (they drop it and system remains stable),
> > but it creates a panic in xen_netfront (xennet_start_xmit()
> > hits a null pointer deref when xennet_make_first_txreq() returns
> > NULL)
> >
> > I'm new to the xen driver code, so I'm hoping that
> > the experts can comment here: reading the code in xennet_start_xmit,
> > it seems like it mandatorily requires the skb_headlen() to be
> > non-zero in order to create the first_tx? That may not always be
> > true, how does the code recover for purely non-linear skbs?
Hi Sowmini,
Sounds like a straightforward bug to me... netfront should be able to handle an empty skb and clearly, if it's relying on skb_headlen() being non-zero, that's not the case.
Paul
> >
> > --Sowmini
>
> CC-ing the two folks from the MAINTAINERS file.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH] net/mlx5e: Remove unused variable
From: Tariq Toukan @ 2017-01-19 9:44 UTC (permalink / raw)
To: Arnd Bergmann, Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: David S. Miller, Gal Pressman, Maor Gottlieb, Eran Ben Elisha,
netdev, linux-rdma, linux-kernel
In-Reply-To: <20170119093401.2582672-1-arnd@arndb.de>
Hi Arnd,
On 19/01/2017 11:33 AM, Arnd Bergmann wrote:
> A cleanup removed the only user of this variable
>
> mlx5/core/en_ethtool.c: In function 'mlx5e_set_channels':
> mlx5/core/en_ethtool.c:546:6: error: unused variable 'ncv' [-Werror=unused-variable]
>
> Let's remove the declaration as well.
>
> Fixes: 639e9e94160e ("net/mlx5e: Remove unnecessary checks when setting num channels")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index b1b9eb6ee135..5197817e4b2f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -543,7 +543,6 @@ static int mlx5e_set_channels(struct net_device *dev,
> struct ethtool_channels *ch)
> {
> struct mlx5e_priv *priv = netdev_priv(dev);
> - int ncv = mlx5e_get_max_num_channels(priv->mdev);
> unsigned int count = ch->combined_count;
> bool arfs_enabled;
> bool was_opened;
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Thanks for the fix!
Regards,
Tariq
^ permalink raw reply
* [PATCH net-next] net/sched: cls_flower: reduce fl_change stack size
From: Arnd Bergmann @ 2017-01-19 9:45 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Jamal Hadi Salim, Jiri Pirko, Hadar Hen Zion,
Amir Vadai, Paul Blakey, Or Gerlitz, Simon Horman, Roi Dayan,
netdev, linux-kernel
The new ARP support has pushed the stack size over the edge on ARM,
as there are two large objects on the stack in this function (mask
and tb) and both have now grown a bit more:
net/sched/cls_flower.c: In function 'fl_change':
net/sched/cls_flower.c:928:1: error: the frame size of 1072 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
We can solve this by dynamically allocating one or both of them.
I first tried to do it just for the mask, but that only saved
152 bytes on ARM, while this version just does it for the 'tb'
array, bringing the stack size back down to 664 bytes.
Fixes: 99d31326cbe6 ("net/sched: cls_flower: Support matching on ARP")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/sched/cls_flower.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 27934456d984..9e74b0fa4b89 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -832,23 +832,31 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
struct cls_fl_head *head = rtnl_dereference(tp->root);
struct cls_fl_filter *fold = (struct cls_fl_filter *) *arg;
struct cls_fl_filter *fnew;
- struct nlattr *tb[TCA_FLOWER_MAX + 1];
+ struct nlattr **tb;
struct fl_flow_mask mask = {};
int err;
if (!tca[TCA_OPTIONS])
return -EINVAL;
+ tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
+ if (!tb)
+ return -ENOBUFS;
+
err = nla_parse_nested(tb, TCA_FLOWER_MAX, tca[TCA_OPTIONS], fl_policy);
if (err < 0)
- return err;
+ goto errout_tb;
- if (fold && handle && fold->handle != handle)
- return -EINVAL;
+ if (fold && handle && fold->handle != handle) {
+ err = -EINVAL;
+ goto errout_tb;
+ }
fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
- if (!fnew)
- return -ENOBUFS;
+ if (!fnew) {
+ err = -ENOBUFS;
+ goto errout_tb;
+ }
err = tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
if (err < 0)
@@ -919,11 +927,14 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
list_add_tail_rcu(&fnew->list, &head->filters);
}
+ kfree(tb);
return 0;
errout:
tcf_exts_destroy(&fnew->exts);
kfree(fnew);
+errout_tb:
+ kfree(tb);
return err;
}
--
2.9.0
^ permalink raw reply related
* Re: [PATCH net-next v4] bridge: multicast to unicast
From: Nikolay Aleksandrov via Bridge @ 2017-01-19 9:53 UTC (permalink / raw)
To: Linus Lüssing, netdev
Cc: bridge, linux-wireless, linux-kernel, David S . Miller,
Felix Fietkau
In-Reply-To: <20170119024510.3284-1-linus.luessing@c0d3.blue>
On 19/01/17 03:45, Linus Lüssing wrote:
> From: Felix Fietkau <nbd@nbd.name>
>
> Implements an optional, per bridge port flag and feature to deliver
> multicast packets to any host on the according port via unicast
> individually. This is done by copying the packet per host and
> changing the multicast destination MAC to a unicast one accordingly.
>
> multicast-to-unicast works on top of the multicast snooping feature of
> the bridge. Which means unicast copies are only delivered to hosts which
> are interested in it and signalized this via IGMP/MLD reports
> previously.
>
> This feature is intended for interface types which have a more reliable
> and/or efficient way to deliver unicast packets than broadcast ones
> (e.g. wifi).
>
> However, it should only be enabled on interfaces where no IGMPv2/MLDv1
> report suppression takes place. This feature is disabled by default.
>
> The initial patch and idea is from Felix Fietkau.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> [linus.luessing@c0d3.blue: various bug + style fixes, commit message]
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
>
> ---
>
> This feature is used and enabled by default in OpenWRT and LEDE for AP
> interfaces for more than a year now to allow both a more robust multicast
> delivery and multicast at higher rates (e.g. multicast streaming).
>
> In OpenWRT/LEDE the IGMP/MLD report suppression issue is overcome by
> the network daemon enabling AP isolation and by that separating all STAs.
> Delivery of STA-to-STA IP mulitcast is made possible again by
> enabling and utilizing the bridge hairpin mode, which considers the
> incoming port as a potential outgoing port, too.
>
> Hairpin-mode is performed after multicast snooping, therefore leading to
> only deliver reports to STAs running a multicast router.
>
> Changes in v4:
> * readd "From: Felix Fietkau [...]" (missed it again in v3)
>
> Changes in v3:
> * fix an uninitialized variable bug introduced in br_multicast_flood()
> in v2, found by kbuild test bot
>
> Changes in v2:
> * netlink support (thanks Nik!)
> * fixed switching between multicast_to_unicast on/off
> -> even after toggling an already existing entry would
> stale in its mode and would never be replaced
> -> new extra check in br_port_group_equal)
> * reduced checks in br_multicast_flood() from two to one
> to address fast-path concerns (thanks Nik!)
> * rev-christmas tree ordering (thanks Nik!)
> * removed "net_bridge_port_group::unicast", using
> ::flags instead (thanks Nik!)
> * BR_MULTICAST_TO_UCAST -> BR_MULTICAST_TO_UNICAST
> (BR_MULTICAST_FLAST_LEAVE has the same length anyway)
> * simplified maybe_deliver_addr()
> (no return, no "prev" paramater -> was a NOP anyway)
> * added "From: Felix Fietkau [...]"
> * added "Signed-off-by: Felix Fietkau [...]"
> ---
> include/linux/if_bridge.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> net/bridge/br_forward.c | 37 ++++++++++++++++-
> net/bridge/br_mdb.c | 2 +-
> net/bridge/br_multicast.c | 96 ++++++++++++++++++++++++++++++++------------
> net/bridge/br_netlink.c | 5 +++
> net/bridge/br_private.h | 8 ++--
> net/bridge/br_sysfs_if.c | 2 +
> 8 files changed, 121 insertions(+), 31 deletions(-)
>
Looks good to me,
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Zefir Kurtisi @ 2017-01-19 9:43 UTC (permalink / raw)
To: Timur Tabi, netdev; +Cc: andrew, f.fainelli
In-Reply-To: <7d61d23d-60f5-a77e-42ad-fccf6e8d5878@codeaurora.org>
On 01/18/2017 04:02 PM, Timur Tabi wrote:
> On 01/18/2017 07:53 AM, Zefir Kurtisi wrote:
>> No, not necessarily. The SGMII link default configuration is set such that you do
>> not have to bother at all.
>
> Does the SGMII link need to make the speed and duplex of the external link?
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c#n50
>
>
> Here is where I can configure the SGMII link to match whatever phydev says the
> external link is. This is code that was handed down to me. I've never really
> understood what the purpose was. Shouldn't I just be able to configure the SGMII
> link to 1Gbs full-duplex, regardless of what the external link is set to?
>
This is because the SGMII link is a transparent interface to the upper layer with
no means to inform the other side of the link type in the lower layer.
It always operates at 675MHz, which with two lines gives 1.25Gbps, which at 10/8
coding gives exactly 1Gbps net data rate. If the at803x's copper side
autonegotiates to 1Gbps, the bits traversing over the SGMII match the copper side
1:1. In case the copper side autonegotiates to e.g. 100Mbps, each bit at the
copper side on the SGMII bus is replicated and sent 10x times - or 100x times in
case of 10Mbps. The MAC side of the ETH needs to be aware of how the SGMII data
has to be interpreted, and this is why you have to set the bits you are referring to.
>> That is, if in your case you see the warning popping up but the link always
>> regains connection, then it is an ignorable false positive.
>
> Unfortunately, I can't really ignore it because genphy does this:
>
> $ ifup eth1
> [ 1034.754276] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow control
> rx/tx
>
> But the at803x driver does this:
>
> $ ifup eth1
> [ 1020.507728] 803x_aneg_done: SGMII link is not ok
> [ 1020.513517] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow control
> rx/tx
> [ 1020.522839] qcom-emac QCOM8070:00 eth1: Link is Up - 1Gbps/Full - flow control
> rx/tx
>
> Customers are going to complain.
>
Yes, the double link-status message is annoying - I was referring to the other
message.
To track down who is causing the additional message, I would proceed with
following technique that helped me dig down a similar problem: since you control
the events in question and there is no risk of flooding the kernel log, in the top
of phy.c::phy_print_status() add a dump_stack() call. In the debug log ensure that
all of the traces end up in the same phydev->adjust_link() callback handler (in
your case emac_adjust_link()).
In the gianfar's handler there is an additional check whether the link really
changed before phy_print_status() is called, in emac_adjust_link() this happens
unconditionally - maybe that is the problem you are facing.
@Florian: is it safe to assume that when phydev->adjust_link() is called there was
in fact a link change (link, duplex, pause), or does the handler have to double
check for a change? I see some ETH drivers maintain a private old state instance
for that, while others don't.
Cheers,
Zefir
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: reduce fl_change stack size
From: Jiri Pirko @ 2017-01-19 9:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Jamal Hadi Salim, Jiri Pirko, Hadar Hen Zion,
Amir Vadai, Paul Blakey, Or Gerlitz, Simon Horman, Roi Dayan,
netdev, linux-kernel
In-Reply-To: <20170119094551.2780983-1-arnd@arndb.de>
Thu, Jan 19, 2017 at 10:45:31AM CET, arnd@arndb.de wrote:
>The new ARP support has pushed the stack size over the edge on ARM,
>as there are two large objects on the stack in this function (mask
>and tb) and both have now grown a bit more:
>
>net/sched/cls_flower.c: In function 'fl_change':
>net/sched/cls_flower.c:928:1: error: the frame size of 1072 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
>We can solve this by dynamically allocating one or both of them.
>I first tried to do it just for the mask, but that only saved
>152 bytes on ARM, while this version just does it for the 'tb'
>array, bringing the stack size back down to 664 bytes.
>
>Fixes: 99d31326cbe6 ("net/sched: cls_flower: Support matching on ARP")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH v3] net/irda: fix lockdep annotation
From: Dmitry Vyukov @ 2017-01-19 10:05 UTC (permalink / raw)
To: David Miller
Cc: Dave Jones, Samuel Ortiz, Alexander Potapenko, andreyknvl, netdev
In-Reply-To: <20170117.151901.998994334183973013.davem@davemloft.net>
On Tue, Jan 17, 2017 at 9:19 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@google.com>
> Date: Mon, 16 Jan 2017 22:10:52 +0100
>
>> The current annotation uses a global variable as recursion counter.
>> The variable is not atomic nor protected with a mutex, but mutated
>> by multiple threads. This causes lockdep bug reports episodically:
>>
>> BUG: looking up invalid subclass: 4294967295
>> ...
>> _raw_spin_lock_irqsave_nested+0x120/0x180
>> hashbin_delete+0x4fe/0x750
>> __irias_delete_object+0xab/0x170
>> irias_delete_object+0x5f/0xc0
>> ircomm_tty_detach_cable+0x1d5/0x3f0
>> ...
>>
>> Make the hashbin_lock_depth variable atomic to prevent bug reports.
>>
>> As is this causes "unused variable 'depth'" warning without LOCKDEP.
>> So also change raw_spin_lock_irqsave_nested() macro to not cause
>> the warning without LOCKDEP. Similar to what raw_spin_lock_nested()
>> already does.
>>
>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Dave Jones <davej@redhat.com>
>> Cc: Samuel Ortiz <samuel@sortiz.org>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: netdev@vger.kernel.org
>> Fixes: c7630a4b932af ("[IrDA]: irda lockdep annotation")
>
> I took a closer look at this, and really this whole lockdep thing is
> more than a hack.
>
> The basic problem is that the free_func() passed into hashbin_delete()
> can trigger hashbin operations on other hashtables.
>
> This function is destroying a hash table completely, and is doing so
> in a context where no new hash table entries can be inserted. The
> last thing we do is kfree() the hashtable so this must be true.
>
> Therefore, it is much better to just kill off all of this lockdep
> stuff, and recode the teardown loop into something like:
>
> if (hashbin->hb_type & HB_LOCK)
> spin_lock_irqsave(&hashbin->hb_lock, flags);
> for (i = 0; i < HASHBIN_SIZE; i++) {
> while (1) {
> queue = dequeue_first((irda_queue_t **) &hashbin->hb_queue[i]);
>
> if (!queue)
> break;
> if (free_func) {
> if (hashbin->hb_type & HB_LOCK)
> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
> free_func(queue);
> if (hashbin->hb_type & HB_LOCK)
> spin_lock_irqsave(&hashbin->hb_lock, flags);
> }
>
> }
> }
> hashbin->hb_current = NULL;
> hashbin->magic = ~HB_MAGIC;
> if (hashbin->hb_type & HB_LOCK)
> spin_unlock_irqrestore(&hashbin->hb_lock, flags);
>
> At which point the recursive locking becomes impossible.
Thanks for looking into it! This particular issue bothers my fuzzers
considerably. I agree that removing recursion is better.
So do how we proceed? Will you mail this as a real patch?
^ 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