* Re: [PATCH net] ipv6: seg6_genl_set_tunsrc() must check kmemdup() return value
From: David Miller @ 2017-01-20 16:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, david.lebrun
In-Reply-To: <1484927862.16328.123.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 20 Jan 2017 07:57:42 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> seg6_genl_get_tunsrc() and set_tun_src() do not handle tun_src being
> possibly NULL, so we must check kmemdup() return value and abort if
> it is NULL
>
> Fixes: 915d7e5e5930 ("ipv6: sr: add code base for control plane support of SR-IPv6")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] stmicro: add more information to Kconfig
From: Joao Pinto @ 2017-01-20 16:00 UTC (permalink / raw)
To: davem; +Cc: rayagond, netdev
In-Reply-To: <8e4b7bbd48a0b2ff1374fdffe6d07dc00659e951.1484664500.git.jpinto@synopsys.com>
My e-mail client went bogus, please ignore 2 past e-mails.
Very sorry.
Best Regards,
Joao Pinto
Às 3:57 PM de 1/20/2017, Joao Pinto escreveu:
> This patch adds more info to stmicro' Kconfig files in order to be clearer
> that the driver can be used by ethernet cards based on 10/100/1000/EQOS
> Synopsys IP Cores.
>
> EQOS was also added stmmac/Kconfig Kconfig, since dwmac4 is in fact EQoS,
> one of Synopsys Ethernet IPs. More info at:
> https://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_qos
>
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> drivers/net/ethernet/stmicro/Kconfig | 3 ++-
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 15 +++++++--------
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/Kconfig b/drivers/net/ethernet/stmicro/Kconfig
> index 1c1157d..ecd7a5e 100644
> --- a/drivers/net/ethernet/stmicro/Kconfig
> +++ b/drivers/net/ethernet/stmicro/Kconfig
> @@ -7,7 +7,8 @@ config NET_VENDOR_STMICRO
> default y
> depends on HAS_IOMEM
> ---help---
> - If you have a network (Ethernet) card belonging to this class, say Y.
> + If you have a network (Ethernet) card based on Synopsys Ethernet IP
> + Cores, say Y.
>
> Note that the answer to this question doesn't directly affect the
> kernel: saying N will just cause the configurator to skip all
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 99594e3..cfbe363 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -1,5 +1,5 @@
> config STMMAC_ETH
> - tristate "STMicroelectronics 10/100/1000 Ethernet driver"
> + tristate "STMicroelectronics 10/100/1000/EQOS Ethernet driver"
> depends on HAS_IOMEM && HAS_DMA
> select MII
> select PHYLIB
> @@ -7,9 +7,8 @@ config STMMAC_ETH
> imply PTP_1588_CLOCK
> select RESET_CONTROLLER
> ---help---
> - This is the driver for the Ethernet IPs are built around a
> - Synopsys IP Core and only tested on the STMicroelectronics
> - platforms.
> + This is the driver for the Ethernet IPs built around a
> + Synopsys IP Core.
>
> if STMMAC_ETH
>
> @@ -152,11 +151,11 @@ config STMMAC_PCI
> tristate "STMMAC PCI bus support"
> depends on STMMAC_ETH && PCI
> ---help---
> - This is to select the Synopsys DWMAC available on PCI devices,
> - if you have a controller with this interface, say Y or M here.
> + This selects the platform specific bus support for the stmmac driver.
> + This driver was tested on XLINX XC2V3000 FF1152AMT0221
> + D1215994A VIRTEX FPGA board and SNPS QoS IPK Prototyping Kit.
>
> - This PCI support is tested on XLINX XC2V3000 FF1152AMT0221
> - D1215994A VIRTEX FPGA board.
> + If you have a controller with this interface, say Y or M here.
>
> If unsure, say N.
> endif
>
^ permalink raw reply
* Re: [PATCH 1/3] cxgb4: hide unused warnings
From: David Miller @ 2017-01-20 16:15 UTC (permalink / raw)
To: arnd; +Cc: hariprasad, ganeshgr, rahul.lakkireddy, varun, netdev,
linux-kernel
In-Reply-To: <20170118145306.1004008-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Jan 2017 15:52:51 +0100
> The two new variables are only used inside of an #ifdef and cause
> harmless warnings when that is disabled:
>
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c: In function 'init_one':
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:4646:9: error: unused variable 'port_vec' [-Werror=unused-variable]
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:4646:6: error: unused variable 'v' [-Werror=unused-variable]
>
> This adds another #ifdef around the declarations.
>
> Fixes: 96fe11f27b70 ("cxgb4: Implement ndo_get_phys_port_id for mgmt dev")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [RFC PATCH] net: stmicro: eQOS IP Core
From: David Miller @ 2017-01-20 16:20 UTC (permalink / raw)
To: Joao.Pinto; +Cc: peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <4584830fd1416af69ddb2e0168f78b1e343c33fe.1484914583.git.jpinto@synopsys.com>
From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Fri, 20 Jan 2017 12:27:33 +0000
> For historical reasons the Designware eQOS IP Core started in version 4.x,
> instead of starting in 1.x. This caused some misunderstanding, which resulted
> in calling this IP as GMAC4. If you go to Synopsys website and check the
> Ethernet portfolio there is no GMAC4 IP:
> https://www.synopsys.com/designware-ip/interface-ip/ethernet.html
>
> There is a new version of eQOS comming out (5.x) that will have the same
> mac, dma registers and flow, with an extra set of registers to enable
> features like TSN. The creation of a dwmac5 or gmac5 would be wrong, and
> including those new features related to eQOS 5.x in dwmac4 would also not
> be very correct.
>
> This patch suggests to rename dwmac4_* driver to eqos_*, turning it the
> official driver for eQOS IP. The registers definitions are also now
> called QOS_ instead of GMAC4_.
>
> For more information about eQOS IP please check:
> https://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_qos
>
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
Sorry, you're not renaming every function in the driver that meets this
qualification.
I understand you want to work hard on the driver, but part of that is
_accepting_ things how they have been for years and making code
improvements and additions rather than messing around playing
rename-this/rename-that games until it looks how you want it to.
Please stop this charade of renaming, I'm getting really tired of it and
I think you really have no concept what kind of effect it has on people
who have to review, apply, and backport this code.
Kconfig text changes and clarifications, that's fine. But all of these
function and variable name changes to the driver, and comment sweeps, no
it is not acceptable to me.
^ permalink raw reply
* [PATCH net-next] bnx2x: avoid two atomic ops per page on x86
From: Eric Dumazet @ 2017-01-20 16:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Gabriel Krisman Bertazi, Yuval Mintz, Ariel Elior
From: Eric Dumazet <edumazet@google.com>
Commit 4cace675d687 ("bnx2x: Alloc 4k fragment for each rx ring buffer
element") added extra put_page() and get_page() calls on arches where
PAGE_SIZE=4K like x86
Reorder things to avoid this overhead.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>
Cc: Yuval Mintz <Yuval.Mintz@cavium.com>
Cc: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 15 ++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 3e199d3e461ef5dd237e3176bcb666f878ac..c0dac0e5696d7d01457a42c2c13f1640c2e2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -549,14 +549,7 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
struct bnx2x_alloc_pool *pool = &fp->page_pool;
dma_addr_t mapping;
- if (!pool->page || (PAGE_SIZE - pool->offset) < SGE_PAGE_SIZE) {
-
- /* put page reference used by the memory pool, since we
- * won't be using this page as the mempool anymore.
- */
- if (pool->page)
- put_page(pool->page);
-
+ if (!pool->page) {
pool->page = alloc_pages(gfp_mask, PAGES_PER_SGE_SHIFT);
if (unlikely(!pool->page))
return -ENOMEM;
@@ -571,7 +564,6 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
return -ENOMEM;
}
- get_page(pool->page);
sw_buf->page = pool->page;
sw_buf->offset = pool->offset;
@@ -581,7 +573,10 @@ static int bnx2x_alloc_rx_sge(struct bnx2x *bp, struct bnx2x_fastpath *fp,
sge->addr_lo = cpu_to_le32(U64_LO(mapping));
pool->offset += SGE_PAGE_SIZE;
-
+ if (PAGE_SIZE - pool->offset >= SGE_PAGE_SIZE)
+ get_page(pool->page);
+ else
+ pool->page = NULL;
return 0;
}
^ permalink raw reply related
* Re: [PATCH net-next] cxgb4: Fix unused variable warning
From: David Miller @ 2017-01-20 16:16 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, hariprasad
In-Reply-To: <1484903839-5890-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 20 Jan 2017 14:47:19 +0530
> Fix unused variable warning when CONFIG_PCI_IOV is not
> defined.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Arnd already submitted a fix for this, and unlike your's his provided
a proper Fixes-by: tag.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:26 UTC (permalink / raw)
To: colin.king; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120130157.20034-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 20 Jan 2017 13:01:57 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparison on the timeout can lead to an array overrun
> read on sctp_timer_tbl because of an off-by-one error. Fix
> this by using < instead of <= and also compare to the array
> size rather than SCTP_EVENT_TIMEOUT_MAX.
>
> Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: remove bh disabling around percpu_counter accesses
From: David Miller @ 2017-01-20 16:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1484917568.16328.110.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 20 Jan 2017 05:06:08 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Shaohua Li made percpu_counter irq safe in commit 098faf5805c8
> ("percpu_counter: make APIs irq safe")
>
> We can safely remove BH disable/enable sections around various
> percpu_counter manipulations.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks great, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8192x: Enabling and disabling hardware interrupts after enabling local irq flags
From: Larry Finger @ 2017-01-20 16:30 UTC (permalink / raw)
To: Bharat Kumar Gogada, chaoming_li@realsil.com.cn,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: kvalo@codeaurora.org, netdev@vger.kernel.org, Ravikiran Gummaluri
In-Reply-To: <8520D5D51A55D047800579B094147198263CA86D@XAP-PVEXMBX02.xlnx.xilinx.com>
On 01/20/2017 08:14 AM, Bharat Kumar Gogada wrote:
> > On 01/19/2017 04:14 AM, Bharat Kumar Gogada wrote:
>>> -Realtek 8192CE chipset maintains local irq flags after enabling/disabling
>>> hardware interrupts.
>>> -Hardware interrupts are enabled before enabling the local irq
>>> flags(these flags are being checked in interrupt handler),
>>> leading to race condition on some RP, where the irq line between
>>> bridge and GIC goes high at ASSERT_INTx and goes low only
>>> at DEASSERT_INTx. In this kind of RP by the time ASSERT_INTx is seen
>>> irq_enable flag is still set to false, resulting in continuous
>>> interrupts seen by CPU as DEASSERT_INTx cannot be sent since
>>> flag is still false and making CPU stall.
>>> -Changing the sequence of setting these irq flags.
>>>
>>> Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com>
>>> ---
>>
>> This patch should be enhanced with the smb_xx() calls as suggested by by Lino.
>>
>> The subject should be changed. I would suggest something like "rtlwifi:
>> rtl8192ce: Prevent race condition when enabling interrupts", as it explains the
>> condition you are preventing.
>>
>> The other PCI drivers also have the same problem. Do you want to prepare the
>> patches, or should I do it?
>>
> Thanks Larry. Please send out the patches adding the above enhancements suggested by Lino.
I have prepared a patch fixing all the drivers. By the way, what CPU hardware
showed this problem?
Larry
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:31 UTC (permalink / raw)
To: colin.king; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120134542.21104-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 20 Jan 2017 13:45:42 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> add this in. Also compare timeout with the size of the array
> sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX. Also add
> a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> so we don't ever get this kind of mismatch between the table
> and SCTP_EVENT_TIMEOUT_MAX in the future.
>
> Kudos to Marcel Ricardo Leitner for spotting the missing string
> and suggesting the build time sanity check.
>
> Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Well, my bad... I reverted V1, that's fine.
But this patch doesn't even compile.
In file included from ./include/uapi/linux/stddef.h:1:0,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from ./include/net/sctp/sctp.h:58,
from net/sctp/debug.c:41:
net/sctp/debug.c: In function ‘sctp_tname’:
./include/linux/compiler.h:518:38: error: call to ‘__compiletime_assert_170’ declared with attribute error: BUILD_BUG_ON failed: SCTP_EVENT_TIMEOUT_MAX + 1 != ARRAY_SIZE(sctp_timer_tbl)
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^ permalink raw reply
* Re: [PATCH net-next] uapi: fix signess in ethtool_validate_speed()
From: Michael S. Tsirkin @ 2017-01-20 16:36 UTC (permalink / raw)
To: Volodymyr Bendiuga
Cc: davem, nikolay, netdev, volodymyr.bendiuga, Jonas Johansson
In-Reply-To: <1484918453-5849-1-git-send-email-volodymyr.bendiuga@gmail.com>
On Fri, Jan 20, 2017 at 02:20:53PM +0100, Volodymyr Bendiuga wrote:
> From: Jonas Johansson <jonas.johansson@westermo.se>
>
> There is a comparison of speed variable which
> is unsigned, and SPEED_UNKNOWN which is signed.
So?
>
> Signed-off-by: Jonas Johansson <jonas.johansson@westermo.se>
> Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@gmail.com>
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index f0db778..1ca4a77 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1500,7 +1500,7 @@ enum ethtool_link_mode_bit_indices {
>
> #define SPEED_UNKNOWN -1
>
> -static inline int ethtool_validate_speed(__u32 speed)
> +static inline int ethtool_validate_speed(__s32 speed)
> {
> return speed <= INT_MAX || speed == SPEED_UNKNOWN;
> }
Then comparison to INT_MAX does not make sense.
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH net-next] gre6: Clean up unused struct ipv6_tel_txoption definition
From: David Miller @ 2017-01-20 16:37 UTC (permalink / raw)
To: jkbs; +Cc: netdev
In-Reply-To: <1484920386-9058-1-git-send-email-jkbs@redhat.com>
From: Jakub Sitnicki <jkbs@redhat.com>
Date: Fri, 20 Jan 2017 14:53:06 +0100
> Commit b05229f44228 ("gre6: Cleanup GREv6 transmit path, call common GRE
> functions") removed the ip6gre specific transmit function, but left the
> struct ipv6_tel_txoption definition. Clean it up.
>
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] sock: use hlist_entry_safe
From: David Miller @ 2017-01-20 16:39 UTC (permalink / raw)
To: geliangtang; +Cc: netdev, linux-kernel
In-Reply-To: <697f2692562e48afa03304a7e605e8adbee8437e.1484829069.git.geliangtang@gmail.com>
From: Geliang Tang <geliangtang@gmail.com>
Date: Fri, 20 Jan 2017 22:27:04 +0800
> Use hlist_entry_safe() instead of open-coding it.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: Marcelo Ricardo Leitner @ 2017-01-20 16:39 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026900F@AcuExch.aculab.com>
On Fri, Jan 20, 2017 at 02:50:01PM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > This patch is to define Add Incoming/Outgoing Streams Request
> > Parameter described in rfc6525 section 4.5 and 4.6. They can
> > be in one same chunk trunk as rfc6525 section 3.1-7 describes,
> > so make them in one function.
> ...
> > +struct sctp_strreset_addstrm {
> > + sctp_paramhdr_t param_hdr;
> > + __u32 request_seq;
> > + __u16 number_of_streams;
> > + __u16 reserved;
> > +} __packed;
> ...
> > + addstrm.param_hdr.type = SCTP_PARAM_RESET_ADD_OUT_STREAMS;
> > + addstrm.param_hdr.length = htons(size);
> > + addstrm.number_of_streams = htons(out);
> > + addstrm.request_seq = htonl(asoc->strreset_outseq);
> > + addstrm.reserved = 0;
> > +
> > + sctp_addto_chunk(retval, size, &addstrm);
>
> Since you allocate the sctp_strreset_addstrm structure on stack
> there is no requirement for it to be packed.
It shouldn't matter that it's allocated on stack. Why should it?
We need it to be packed as this is a header that will be sent out to
another peer, so there can't be any padding on it.
Marcelo
^ permalink raw reply
* Re: [PATCH net-next V4] tc: flower: Refactor matching flags to be more user friendly
From: Jiri Benc @ 2017-01-20 16:41 UTC (permalink / raw)
To: David Laight
Cc: Paul Blakey, Stephen Hemminger, netdev@vger.kernel.org,
Jiri Pirko, Or Gerlitz, Roi Dayan, Simon Horman
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0268D18@AcuExch.aculab.com>
On Fri, 20 Jan 2017 12:27:42 +0000, David Laight wrote:
> Consider what happened with "no", "nofubar" and "nofubar_baz",
> all ought to be rejected.
Why? "no" translates to "", "nofubar" to "fubar", etc. And those will
be evaluated the same way as if they were supplied without the "no".
I don't see a problem with this.
> Actually using strncmp() is also overkill.
Why? It compares two bytes. There's an extra null at the end of the
"no" string but I wouldn't call that "overkill".
> Nothing wrong with:
> if (token[0] == 'n' && token[1] == 'o' && token[2]) {
Except that strncmp is easier to understand and cleaner.
> no = true;
> token += 2;
> if (token[0] == '_' && token[1])
> token++;
This doesn't make sense. The intent was not to allow both "nofrag" and
"no_frag". The code in the patch treats "no_frag" as invalid and that's
okay.
> ...
>
> or replace the last 3 lines with:
> token += 2 + (token[2] == '_' & token[3]);
That's horribly ugly. Anyone looking at this will spent 2 minutes
trying to untangle the code instead of the 2 seconds with the current
code. We're not trying to win the Obfuscated C Contest here.
Jiri
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:32 UTC (permalink / raw)
To: marcelo.leitner
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120135845.GT3781@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri, 20 Jan 2017 11:58:45 -0200
> Not sure I can add the Fixes tag for you here, but:
> Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
FWIW, patchwork doesn't notice updated Fixes: tags like this but I
try to do so myself and the effort is definitely appreciated.
We need a V3 of this change anyways, see the build failure I reported
for V2, so Colin can add the Fixes: tag there explicitly.
^ permalink raw reply
* Re: Getting a handle on all these new NIC features
From: Martin Habets @ 2017-01-20 16:36 UTC (permalink / raw)
To: Tom Herbert, Linux Kernel Network Developers
In-Reply-To: <CALx6S37YLY7-9YUTWBCBuLCWxbQL1=YURzqFZ00S1pb=2W9kFg@mail.gmail.com>
Hi Tom,
On 17/01/17 22:05, Tom Herbert wrote:
> There was some discussion about the problems of dealing with the
> explosion of NIC features in the mlx directory restructuring proposal,
> but I think the is a deeper issue here that should be discussed.
>
> It's hard not to notice that there has been quite a proliferation of
> NIC features in several drivers. This trend had resulted in very
> complex driver code that may or may not segment individual features.
> One visible manifestation of this is number of ndo functions which is
> somewhere around seventy-five now.
>
> I suspect the vast majority of these advances NIC features (e.g.
> bridging, UDP offloads, tc offload, etc.) are only relevant to some of
> the people some of the time. The problem we have, in this case those
> of us that are attempting to deploy and maintain NICs at scale, is
> when we have to deal with the ramifications of these features being
> intertwined with core driver functionality that is relevant to
> everyone. This becomes very obvious when we need to backport drivers
> from later versions of kernel.
>
> I realize that backports of a driver is not a specific concern of the
> Linux kernel, but nevertheless this is a real problem and a fact of
> life for many users. Rebasing the full kernel is still a major effort
> and it seems the best we could ever do is one rebase per year. In the
> interim we need to occasionally backport drivers. Backporting drivers
> is difficult precisely because of new features or API changes to
> existing ones. These sort of changes tend to have a spiderweb of
> dependencies in other parts of the stack so that the number of patches
> we need to cherry-pick goes way beyond those that touch the driver we
> are interested in.
For the sfc driver (Solarflare Adapters) we currently do backports internally for:
- RedHat Enterprise Linux 5.10, 5.11
- RedHat Enterprise Linux 6.5, 6.6, 6.7, 6.8
- Redhat Messaging Realtime and Grid 2.5
- RedHat Enterprise Linux 7.0, 7.1, 7.2
- RedHat Enterprise Linux for Realtime 7.1, 7.2
- SuSE Linux Enterprise Server 11 sp3, sp4
- SuSE Linux Enterprise RealTime Extension 11
- SuSE Linux Enterprise Server 12 base release, sp1
- Canonical Ubuntu Server LTS 14.04, 16.04
- Canonical Ubuntu Server -
- Debian 7 "Wheezy" 7.X
- Debian 8 "Jessie" 8.X
- Linux 2.6.18 to 4.9-rc1
We update this list as needed, and always try to support the latest kernel.
I do not know if that would cover the kernel version you are using.
Best regards,
Martin
> Currently we (FB) need to backport two NIC drivers. I've already gave
> details of backporting mlx5 on the thread to restructure the driver
> directories. The other driver being backporting seems to suffer from
> the same type of feature complexity.
>
> In short, I would like to ask if driver maintainers to start to
> modularize driver features. If something being added is obviously a
> narrow feature that only a subset of users will need can we allow
> config options to #ifdef those out somehow? Furthermore can the file
> and directory structure of drivers reflect that; our lives would be
> _so_ much simpler to maintain drivers in production if we have such
> modularity and the ability to build drivers with the features of our
> choosing.
>
> Thanks,
> Tom
^ permalink raw reply
* Re: [net-next PATCH 0/3] Retrieve number of VFs in a bus-agnostic way
From: David Miller @ 2017-01-20 16:43 UTC (permalink / raw)
To: phil; +Cc: bhelgaas, netdev, linux-pci
In-Reply-To: <20170118130439.15023-1-phil@nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 18 Jan 2017 14:04:36 +0100
> Previously, it was assumed that only PCI NICs would be capable of having
> virtual functions - with my proposed enhancement of dummy NIC driver
> implementing (fake) ones for testing purposes, this is no longer true.
>
> Discussion of said patch has led to the suggestion of implementing a
> bus-agnostic method for VF count retrieval so rtnetlink could work with
> both real VF-capable PCI NICs as well as my dummy modifications without
> introducing ugly hacks.
>
> The following series tries to achieve just that by introducing a bus
> type callback to retrieve a device's number of VFs, implementing this
> callback for PCI bus and finally adjusting rtnetlink to make use of the
> generalized infrastructure.
This is really nice and clean, compare it to your original approach :-)
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH 2/3] qed: avoid possible stack overflow in qed_ll2_acquire_connection
From: David Miller @ 2017-01-20 16:44 UTC (permalink / raw)
To: arnd
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, Ram.Amrani,
weiyongjun1, hare, netdev, linux-kernel
In-Reply-To: <20170118145306.1004008-2-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Jan 2017 15:52:52 +0100
> struct qed_ll2_info is rather large, so putting it on the stack
> can cause an overflow, as this warning tries to tell us:
>
> drivers/net/ethernet/qlogic/qed/qed_ll2.c: In function 'qed_ll2_start':
> drivers/net/ethernet/qlogic/qed/qed_ll2.c:2159:1: error: the frame size of 1056 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> qed_ll2_start_ooo() already uses a dynamic allocation for the structure
> to work around that problem, and we could do the same in qed_ll2_start()
> as well as qed_roce_ll2_start(), but since the structure is only
> used to pass a couple of initialization values here, it seems nicer
> to replace it with a different structure.
>
> Lacking any idea for better naming, I'm adding 'struct qed_ll2_conn',
> which now contains all the initialization data, and this now simply
> gets copied into struct qed_ll2_info rather than assigning all members
> one by one.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH net] virtio-net: restore VIRTIO_HDR_F_DATA_VALID on receiving
From: Michael S. Tsirkin @ 2017-01-20 16:45 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, Rolf Neugebauer, linux-kernel, virtualization
In-Reply-To: <1484893962-7614-1-git-send-email-jasowang@redhat.com>
On Fri, Jan 20, 2017 at 02:32:42PM +0800, Jason Wang wrote:
> Commit 501db511397f ("virtio: don't set VIRTIO_NET_HDR_F_DATA_VALID on
> xmit") in fact disables VIRTIO_HDR_F_DATA_VALID on receiving path too,
> fixing this by adding a hint (has_data_valid) and set it only on the
> receiving path.
>
> Cc: Rolf Neugebauer <rolf.neugebauer@docker.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/macvtap.c | 2 +-
> drivers/net/tun.c | 2 +-
> drivers/net/virtio_net.c | 2 +-
> include/linux/virtio_net.h | 6 +++++-
> net/packet/af_packet.c | 4 ++--
> 5 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 5c26653..4026185 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -825,7 +825,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
> return -EINVAL;
>
> if (virtio_net_hdr_from_skb(skb, &vnet_hdr,
> - macvtap_is_little_endian(q)))
> + macvtap_is_little_endian(q), true))
> BUG();
>
> if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index cd8e02c..2cd10b2 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1360,7 +1360,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
> return -EINVAL;
>
> if (virtio_net_hdr_from_skb(skb, &gso,
> - tun_is_little_endian(tun))) {
> + tun_is_little_endian(tun), true)) {
> struct skb_shared_info *sinfo = skb_shinfo(skb);
> pr_err("unexpected GSO type: "
> "0x%x, gso_size %d, hdr_len %d\n",
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4a10500..3474243 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1104,7 +1104,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> hdr = skb_vnet_hdr(skb);
>
> if (virtio_net_hdr_from_skb(skb, &hdr->hdr,
> - virtio_is_little_endian(vi->vdev)))
> + virtio_is_little_endian(vi->vdev), false))
> BUG();
>
> if (vi->mergeable_rx_bufs)
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 5643647..5209b5e 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -56,7 +56,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>
> static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> struct virtio_net_hdr *hdr,
> - bool little_endian)
> + bool little_endian,
> + bool has_data_valid)
> {
> memset(hdr, 0, sizeof(*hdr)); /* no info leak */
>
I would prefer naming it is_rx. Callers should not know about
internal details like data valid, the issue we are trying to fix
here is that tx and tx headers are slightly different.
> @@ -91,6 +92,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> skb_checksum_start_offset(skb));
> hdr->csum_offset = __cpu_to_virtio16(little_endian,
> skb->csum_offset);
> + } else if (has_data_valid &&
> + skb->ip_summed == CHECKSUM_UNNECESSARY) {
> + hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
>
> return 0;
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b9e1a13..3d555c7 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1976,7 +1976,7 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb,
> return -EINVAL;
> *len -= sizeof(vnet_hdr);
>
> - if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le()))
> + if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le(), true))
> return -EINVAL;
>
> return memcpy_to_msg(msg, (void *)&vnet_hdr, sizeof(vnet_hdr));
> @@ -2237,7 +2237,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> if (po->has_vnet_hdr) {
> if (virtio_net_hdr_from_skb(skb, h.raw + macoff -
> sizeof(struct virtio_net_hdr),
> - vio_le())) {
> + vio_le(), true)) {
> spin_lock(&sk->sk_receive_queue.lock);
> goto drop_n_account;
> }
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH 3/3] bcm63xx_enet: avoid uninitialized variable warning
From: David Miller @ 2017-01-20 16:46 UTC (permalink / raw)
To: arnd
Cc: f.fainelli, xypron.glpk, linux-kernel, bcm-kernel-feedback-list,
netdev, linux-arm-kernel, tremyfr
In-Reply-To: <20170118145306.1004008-3-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 18 Jan 2017 15:52:53 +0100
> gcc-7 and probably earlier versions get confused by this function
> and print a harmless warning:
>
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open':
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1130:3: error: 'phydev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds an initialization for the 'phydev' variable when it is unused
> and changes the check to test for that NULL pointer to make it clear
> that we always pass a valid pointer here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Also applied, thanks.
^ permalink raw reply
* [PATCH net] bridge: netlink: call br_changelink() during br_dev_newlink()
From: Ivan Vecera @ 2017-01-20 16:46 UTC (permalink / raw)
To: netdev; +Cc: stephen, davem, bridge, jiri
Any bridge options specified during link creation (e.g. ip link add)
are ignored as br_dev_newlink() does not process them.
Use br_changelink() to do it.
Signed-off-by: Ivan Vecera <cera@cera.cz>
---
net/bridge/br_netlink.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 71c7453..7109b38 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -781,20 +781,6 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[])
return 0;
}
-static int br_dev_newlink(struct net *src_net, struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
-{
- struct net_bridge *br = netdev_priv(dev);
-
- if (tb[IFLA_ADDRESS]) {
- spin_lock_bh(&br->lock);
- br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
- spin_unlock_bh(&br->lock);
- }
-
- return register_netdevice(dev);
-}
-
static int br_port_slave_changelink(struct net_device *brdev,
struct net_device *dev,
struct nlattr *tb[],
@@ -1115,6 +1101,25 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
return 0;
}
+static int br_dev_newlink(struct net *src_net, struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
+ struct net_bridge *br = netdev_priv(dev);
+ int err;
+
+ if (tb[IFLA_ADDRESS]) {
+ spin_lock_bh(&br->lock);
+ br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS]));
+ spin_unlock_bh(&br->lock);
+ }
+
+ err = br_changelink(dev, tb, data);
+ if (err)
+ return err;
+
+ return register_netdevice(dev);
+}
+
static size_t br_get_size(const struct net_device *brdev)
{
return nla_total_size(sizeof(u32)) + /* IFLA_BR_FORWARD_DELAY */
--
2.10.2
^ permalink raw reply related
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: Marcelo Ricardo Leitner @ 2017-01-20 16:48 UTC (permalink / raw)
To: David Miller
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120.113117.1592305434267734738.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:31:17AM -0500, David Miller wrote:
> From: Colin King <colin.king@canonical.com>
> Date: Fri, 20 Jan 2017 13:45:42 +0000
>
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Table sctp_timer_tbl is missing a TIMEOUT_RECONF string so
> > add this in. Also compare timeout with the size of the array
> > sctp_timer_tbl rather than SCTP_EVENT_TIMEOUT_MAX. Also add
> > a build time check that SCTP_EVENT_TIMEOUT_MAX is correct
> > so we don't ever get this kind of mismatch between the table
> > and SCTP_EVENT_TIMEOUT_MAX in the future.
> >
> > Kudos to Marcel Ricardo Leitner for spotting the missing string
> > and suggesting the build time sanity check.
> >
> > Fixes CoverityScan CID#1397639 ("Out-of-bounds read")
> >
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Well, my bad... I reverted V1, that's fine.
>
> But this patch doesn't even compile.
>
> In file included from ./include/uapi/linux/stddef.h:1:0,
> from ./include/linux/stddef.h:4,
> from ./include/uapi/linux/posix_types.h:4,
> from ./include/uapi/linux/types.h:13,
> from ./include/linux/types.h:5,
> from ./include/net/sctp/sctp.h:58,
> from net/sctp/debug.c:41:
> net/sctp/debug.c: In function ‘sctp_tname’:
> ./include/linux/compiler.h:518:38: error: call to ‘__compiletime_assert_170’ declared with attribute error: BUILD_BUG_ON failed: SCTP_EVENT_TIMEOUT_MAX + 1 != ARRAY_SIZE(sctp_timer_tbl)
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
Seems you applied it on net tree, but commit that introduced the issue
(7b9438de0cd4) is still only on net-next.
I build-tested it here before acking, it worked, on top of
4567d686f5c6d955e57a3afa1741944c1e7f4033.
Colin, please respin the patch.. add the Fixes tag, fix the missing 'o'
in my name on the changelog :-) and tag the patch as net-next tree too.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: marcelo.leitner @ 2017-01-20 16:51 UTC (permalink / raw)
To: David Miller
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120.113209.1764653226397979876.davem@davemloft.net>
On Fri, Jan 20, 2017 at 11:32:09AM -0500, David Miller wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date: Fri, 20 Jan 2017 11:58:45 -0200
>
> > Not sure I can add the Fixes tag for you here, but:
> > Fixes: 7b9438de0cd4 ("sctp: add stream reconf timer")
> > Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> FWIW, patchwork doesn't notice updated Fixes: tags like this but I
> try to do so myself and the effort is definitely appreciated.
Ok, good to know, thanks.
>
> We need a V3 of this change anyways, see the build failure I reported
> for V2, so Colin can add the Fixes: tag there explicitly.
>
Replied this one in there.
Marcelo
^ permalink raw reply
* Re: [PATCH][V2] net: sctp: fix array overrun read on sctp_timer_tbl
From: David Miller @ 2017-01-20 16:52 UTC (permalink / raw)
To: marcelo.leitner
Cc: colin.king, vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20170120164820.GA669@localhost.localdomain>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri, 20 Jan 2017 14:48:20 -0200
> Seems you applied it on net tree, but commit that introduced the issue
> (7b9438de0cd4) is still only on net-next.
That explains everything.
> I build-tested it here before acking, it worked, on top of
> 4567d686f5c6d955e57a3afa1741944c1e7f4033.
>
> Colin, please respin the patch.. add the Fixes tag, fix the missing 'o'
> in my name on the changelog :-) and tag the patch as net-next tree too.
Indeed, thanks.
^ 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