* RE: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
From: Chris Mi @ 2018-01-09 1:21 UTC (permalink / raw)
To: Phil Sutter
Cc: dsahern@gmail.com, marcelo.leitner@gmail.com,
netdev@vger.kernel.org, gerlitz.or@gmail.com,
stephen@networkplumber.org
In-Reply-To: <20180108133150.GE14358@orbyte.nwl.cc>
> -----Original Message-----
> From: n0-1@orbyte.nwl.cc [mailto:n0-1@orbyte.nwl.cc] On Behalf Of Phil
> Sutter
> Sent: Monday, January 8, 2018 9:32 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: dsahern@gmail.com; marcelo.leitner@gmail.com;
> netdev@vger.kernel.org; gerlitz.or@gmail.com;
> stephen@networkplumber.org
> Subject: Re: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
>
> Hi Chris,
>
> On Mon, Jan 08, 2018 at 02:03:53AM +0000, Chris Mi wrote:
> > > On Thu, Jan 04, 2018 at 04:34:51PM +0900, Chris Mi wrote:
> > > > The insertion rate is improved more than 10%.
> > >
> > > Did you measure the effect of increasing batch sizes?
> > Yes. Even if we enlarge the batch size bigger than 10, there is no big
> improvement.
> > I think that's because current kernel doesn't process the requests in
> parallel.
> > If kernel processes the requests in parallel, I believe specifying a
> > bigger batch size will get a better result.
>
> But throughput doesn't regress at some point, right? I think that's the critical
> aspect when considering an "unlimited" batch size.
Yes.
>
> On Mon, Jan 08, 2018 at 08:00:00AM +0000, Chris Mi wrote:
> > After testing, I find that the message passed to kernel should not be too
> big.
> > If it is bigger than about 64K, sendmsg returns -1, errno is 90 (EMSGSIZE).
> > That is about 400 commands. So how about set batch size to 128 which is
> big enough?
>
> If that's the easiest way, why not. At first, I thought one could maybe send
> the collected messages in chunks of suitable size, but that's probably not
> worth the effort.
OK.
-Chris
^ permalink raw reply
* Re: [PATCH net 0/3] Some sockopt optlen fixes
From: Neil Horman @ 2018-01-09 1:23 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev, linux-sctp, Vlad Yasevich, haliu
In-Reply-To: <cover.1515436112.git.marcelo.leitner@gmail.com>
On Mon, Jan 08, 2018 at 07:02:26PM -0200, Marcelo Ricardo Leitner wrote:
> Hangbin Liu reported that some SCTP sockopt are allowing the user to get
> the kernel to allocate really large buffers by not having a ceiling on
> optlen.
>
> This patchset address this issue (in patch 2), replace an GFP_ATOMIC
> that isn't needed and avoid calculating the option size multiple times
> in some setsockopt.
>
> Marcelo Ricardo Leitner (3):
> sctp: GFP_ATOMIC is not needed in sctp_setsockopt_events
> sctp: add a ceiling to optlen in some sockopts
> sctp: make use of pre-calculated len
>
> net/sctp/socket.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> --
> 2.14.3
>
>
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Jia-Ju Bai @ 2018-01-09 1:36 UTC (permalink / raw)
To: Larry Finger, Kalle Valo
Cc: kstewart, johannes.berg, tiwai, gregkh, linux-wireless,
linux-kernel, andrew.zaborowski, b43-dev, netdev, colin.king
In-Reply-To: <fc3e2558-c31f-289d-8413-33fef4f5ca64@lwfinger.net>
On 2018/1/9 0:31, Larry Finger wrote:
> On 01/08/2018 10:21 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>>> b43_radio_2057_init_post is not called in an interrupt handler
>>> nor holding a spinlock.
>>> The function mdelay in it can be replaced with msleep, to reduce
>>> busy wait.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>
>> You submitted an identical patch a week earlier:
>>
>> https://patchwork.kernel.org/patch/10137671/
>>
>> How is this different? Also always add version number to the patch so
>> that the
>> maintainers can follow the changes easily:
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
>>
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>>
>
> I had negative comments on one of those due to the possibility of
> msleep(2) extending as long as 20 msec. Until the author, or someone
> else, can test that this is OK, then the mdelay(2) can only be
> replaced with usleep_range(2000, 3000).
>
> NACK for both.
>
> Larry
>
Sorry for my mistake.
I have sent a patch v2 using usleep_range(2000, 3000), and you can have
a look :)
Thanks,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: David Miller @ 2018-01-09 1:39 UTC (permalink / raw)
To: kubakici; +Cc: lipeng321, netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108120431.1e4b45e1@cakuba.netronome.com>
From: Jakub Kicinski <kubakici@wp.pl>
Date: Mon, 8 Jan 2018 12:04:31 -0800
> Ugh, I so didn't review this in time :( I think there is a consensus
> that we should avoid duplicating standard stats in ethtool. Especially
> those old ones. Like "collisions", I assume this is a modern NIC, are
> collisions still a thing?
There is no standard way to get per-queue values, and ethtool stats are
how pretty much every driver provides it.
^ permalink raw reply
* [PATCH] ath9k: add a quirk to set use_msi automatically
From: AceLan Kao @ 2018-01-09 1:39 UTC (permalink / raw)
To: QCA ath9k Development, Kalle Valo, linux-wireless, netdev,
linux-kernel
Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding the following platforms to the quirk.
Dell Inspiron 24-3460
Dell Inspiron 3472
Dell Inspiron 14-3473
Dell Vostro 3262
Dell Vostro 15-3572
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/net/wireless/ath/ath9k/init.c | 53 +++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 43adead..e479fae 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/relay.h>
+#include <linux/dmi.h>
#include <net/ieee80211_radiotap.h>
#include "ath9k.h"
@@ -96,6 +97,56 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
};
#endif
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+ ath9k_use_msi = 1;
+ return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 24-3460",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Vostro 3262",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 3472",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Vostro 15-3572",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 14-3473",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+ },
+ },
+ {}
+};
+
static void ath9k_deinit_softc(struct ath_softc *sc);
static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1155,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
+ dmi_check_system(ath9k_quirks);
+
return 0;
err_pci_exit:
--
2.7.4
^ permalink raw reply related
* [PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post
From: Jia-Ju Bai @ 2018-01-09 1:40 UTC (permalink / raw)
To: Larry.Finger, kvalo, kstewart, johannes.berg, tiwai, gregkh,
colin.king, andrew.zaborowski
Cc: linux-kernel, linux-wireless, netdev, b43-dev, Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: Jakub Kicinski @ 2018-01-09 1:46 UTC (permalink / raw)
To: David Miller; +Cc: lipeng321, netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108.203913.1283013651432416478.davem@davemloft.net>
On Mon, 08 Jan 2018 20:39:13 -0500 (EST), David Miller wrote:
> From: Jakub Kicinski <kubakici@wp.pl>
> Date: Mon, 8 Jan 2018 12:04:31 -0800
>
> > Ugh, I so didn't review this in time :( I think there is a consensus
> > that we should avoid duplicating standard stats in ethtool. Especially
> > those old ones. Like "collisions", I assume this is a modern NIC, are
> > collisions still a thing?
>
> There is no standard way to get per-queue values, and ethtool stats are
> how pretty much every driver provides it.
Right, agreed. I'm only objecting to this patch (12/20), where we can
see the telltale code like this:
+ const struct rtnl_link_stats64 *net_stats;
+ struct rtnl_link_stats64 temp;
+
+ net_stats = dev_get_stats(netdev, &temp);
+ for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) {
+ stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset;
+ *data++ = *(u64 *)stat;
+ }
Where:
+#define HNS3_NETDEV_STAT(_string, _member) { \
+ .stats_string = _string, \
+ .stats_offset = offsetof(struct rtnl_link_stats64, _member) \
+}
+
+static const struct hns3_stats hns3_netdev_stats[] = {
+ /* Rx per-queue statistics */
+ HNS3_NETDEV_STAT("rx_packets", rx_packets),
+ HNS3_NETDEV_STAT("tx_packets", tx_packets),
etc. IOW dumping struct rtnl_link_stats64 to ethtool -S member by
member.
Let me put the netlink per-queue stats on my soft TODO list :)
^ permalink raw reply
* RE: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
From: Chris Mi @ 2018-01-09 1:49 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Ahern, Phil Sutter, marcelo.leitner@gmail.com,
netdev@vger.kernel.org, gerlitz.or@gmail.com
In-Reply-To: <20180108074014.458333d1@xeon-e3>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, January 8, 2018 11:40 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: David Ahern <dsahern@gmail.com>; Phil Sutter <phil@nwl.cc>;
> marcelo.leitner@gmail.com; netdev@vger.kernel.org; gerlitz.or@gmail.com
> Subject: Re: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
>
> On Mon, 8 Jan 2018 08:00:00 +0000
> Chris Mi <chrism@mellanox.com> wrote:
>
> > > >> I wonder whether specifying the batch size is necessary at all.
> > > >> Couldn't batch mode just collect messages until either EOF or an
> > > >> incompatible command is encountered which then triggers a commit
> > > >> to kernel? This might simplify code quite a bit.
> > > > That's a good suggestion.
> > >
> > > Thanks for your time on this, Chris.
> > After testing, I find that the message passed to kernel should not be too
> big.
> > If it is bigger than about 64K, sendmsg returns -1, errno is 90 (EMSGSIZE).
> > That is about 400 commands. So how about set batch size to 128 which is
> big enough?
>
>
> Use sendmmsg?
Maybe we can try that, but there is also a limit on it.
^ permalink raw reply
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: Jakub Kicinski @ 2018-01-09 1:50 UTC (permalink / raw)
To: David Miller; +Cc: lipeng321, netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108174602.34bee66b@cakuba.netronome.com>
On Mon, 8 Jan 2018 17:46:02 -0800, Jakub Kicinski wrote:
> On Mon, 08 Jan 2018 20:39:13 -0500 (EST), David Miller wrote:
> > From: Jakub Kicinski <kubakici@wp.pl>
> > Date: Mon, 8 Jan 2018 12:04:31 -0800
> >
> > > Ugh, I so didn't review this in time :( I think there is a consensus
> > > that we should avoid duplicating standard stats in ethtool. Especially
> > > those old ones. Like "collisions", I assume this is a modern NIC, are
> > > collisions still a thing?
> >
> > There is no standard way to get per-queue values, and ethtool stats are
> > how pretty much every driver provides it.
>
> Right, agreed. I'm only objecting to this patch (12/20), where we can
> see the telltale code like this:
>
> + const struct rtnl_link_stats64 *net_stats;
> + struct rtnl_link_stats64 temp;
> +
> + net_stats = dev_get_stats(netdev, &temp);
> + for (i = 0; i < HNS3_NETDEV_STATS_COUNT; i++) {
> + stat = (u8 *)net_stats + hns3_netdev_stats[i].stats_offset;
> + *data++ = *(u64 *)stat;
> + }
>
> Where:
>
> +#define HNS3_NETDEV_STAT(_string, _member) { \
> + .stats_string = _string, \
> + .stats_offset = offsetof(struct rtnl_link_stats64, _member) \
> +}
> +
> +static const struct hns3_stats hns3_netdev_stats[] = {
> + /* Rx per-queue statistics */
Oh, I only noticed this extra misleading comment now. Unless each queue
has a netdev, I don't see how these are per-queue.
> + HNS3_NETDEV_STAT("rx_packets", rx_packets),
> + HNS3_NETDEV_STAT("tx_packets", tx_packets),
>
> etc. IOW dumping struct rtnl_link_stats64 to ethtool -S member by
> member.
>
> Let me put the netlink per-queue stats on my soft TODO list :)
>
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] Replace WARN_ONCE usages with netdev_WARN_ONCE
From: David Miller @ 2018-01-09 1:53 UTC (permalink / raw)
To: galp; +Cc: netdev, tariqt, saeedm
In-Reply-To: <1515319720-18428-1-git-send-email-galp@mellanox.com>
From: Gal Pressman <galp@mellanox.com>
Date: Sun, 7 Jan 2018 12:08:34 +0200
> This series will fix an issue in netdev_WARN_ONCE, improve its formatting and
> replace drivers' usage of WARN_ONCE to netdev_WARN_ONCE.
>
> Driver specific patches were compilation tested, in addition, functional tested
> on Mellanox NIC.
>
> v1->v2:
> - Addressed commit message comments in patch #1
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: David Miller @ 2018-01-09 1:54 UTC (permalink / raw)
To: kubakici; +Cc: lipeng321, netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108175021.4a769c33@cakuba.netronome.com>
From: Jakub Kicinski <kubakici@wp.pl>
Date: Mon, 8 Jan 2018 17:50:21 -0800
> Oh, I only noticed this extra misleading comment now. Unless each queue
> has a netdev, I don't see how these are per-queue.
If it isn't per-queue I want this change reverted.
^ permalink raw reply
* Re: [PATCH 00/52] Netfilter/IPVS updates for net-next
From: David Miller @ 2018-01-09 1:55 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20180108202000.12989-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 8 Jan 2018 21:19:08 +0100
> The following patchset contains Netfilter/IPVS updates for your
> net-next tree:
...
> 4) Add generic flow table offload infrastructure for nf_tables, this
> includes the netlink control plane and support for IPv4, IPv6 and
> mixed IPv4/IPv6 dataplanes. This comes with NAT support too. This
> patchset adds the IPS_OFFLOAD conntrack status bit to indicate that
> this flow has been offloaded.
Have driver maintainers signed off on your offload design and driver
interfaces?
I've pulled, but the above is really important to indicate when a new
offload feature is added.
Thanks.
^ permalink raw reply
* Re: [RESEND PATCH 1/3] net: caif: remove unused hardirq.h
From: David Miller @ 2018-01-09 1:59 UTC (permalink / raw)
To: yang.s; +Cc: netdev, linux-kernel, dmitry.tarnyagin
In-Reply-To: <1515441174-122496-1-git-send-email-yang.s@alibaba-inc.com>
From: "Yang Shi" <yang.s@alibaba-inc.com>
Date: Tue, 09 Jan 2018 03:52:52 +0800
> Preempt counter APIs have been split out, currently, hardirq.h just
> includes irq_enter/exit APIs which are not used by caif at all.
>
> So, remove the unused hardirq.h.
>
> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
Applied.
^ permalink raw reply
* Re: [RESEND PATCH 2/3] net: ovs: remove unused hardirq.h
From: David Miller @ 2018-01-09 1:59 UTC (permalink / raw)
To: yang.s; +Cc: netdev, linux-kernel, dev
In-Reply-To: <1515441174-122496-2-git-send-email-yang.s@alibaba-inc.com>
From: "Yang Shi" <yang.s@alibaba-inc.com>
Date: Tue, 09 Jan 2018 03:52:53 +0800
> Preempt counter APIs have been split out, currently, hardirq.h just
> includes irq_enter/exit APIs which are not used by openvswitch at all.
>
> So, remove the unused hardirq.h.
>
> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> Acked-by: Pravin B Shelar <pshelar@ovn.org>
Applied.
^ permalink raw reply
* Re: [RESEND PATCH 3/3] net: tipc: remove unused hardirq.h
From: David Miller @ 2018-01-09 1:59 UTC (permalink / raw)
To: yang.s; +Cc: netdev, linux-kernel, jon.maloy
In-Reply-To: <1515441174-122496-3-git-send-email-yang.s@alibaba-inc.com>
From: "Yang Shi" <yang.s@alibaba-inc.com>
Date: Tue, 09 Jan 2018 03:52:54 +0800
> Preempt counter APIs have been split out, currently, hardirq.h just
> includes irq_enter/exit APIs which are not used by TIPC at all.
>
> So, remove the unused hardirq.h.
>
> Signed-off-by: Yang Shi <yang.s@alibaba-inc.com>
> Acked-by: Ying Xue <ying.xue@windriver.com>
> Tested-by: Ying Xue <ying.xue@windriver.com>
Applied.
^ permalink raw reply
* Re: Subject: [RFC][PATCH 04/11] stmmac: fix breakage in stmmac_hw_setup()
From: David Miller @ 2018-01-09 2:02 UTC (permalink / raw)
To: viro; +Cc: netdev, peppe.cavallaro, Cavallaro, Giuseppe
In-Reply-To: <E1eXXig-0002B1-IA@ZenIV.linux.org.uk>
From: Al Viro <viro@ftp.linux.org.uk>
Date: Fri, 05 Jan 2018 19:31:58 +0000
> Since "drivers: net: stmmac: reworking the PCS code" ->pcs_ctrl_ane()
> had been taking iomem address to access as the first argument; its
> predecessor (->ctrl_ane()) used to take struct mac_device_info instead.
>
> One of the callers had not been converted; as the result, instead of
> reading and modifying a word in card iomem we read and modify a word
> in (or near) the in-core strucct mac_device_info.
>
> Fixes: 70523e639bf8 (drivers: net: stmmac: reworking the PCS code)
> Cc: stable@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Yikes...
Al, can you split the bug fixes like this one into a separate series
for me to pull into my net GIT tree?
Don't include the pure annotation or partial endianness conversion
ones, those should go to net-next.
^ permalink raw reply
* Re: [PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution
From: Dan Williams @ 2018-01-09 2:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Linux Kernel Mailing List, linux-arch, Alan Cox, Peter Zijlstra,
Netdev, Greg KH, Thomas Gleixner, Mauro Carvalho Chehab,
Linus Torvalds, Elena Reshetova, Linux-media@vger.kernel.org
In-Reply-To: <8223870.kpF3YQejF4@avalon>
On Mon, Jan 8, 2018 at 3:23 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Dan,
>
> Thank you for the patch.
>
> On Saturday, 6 January 2018 03:10:32 EET Dan Williams wrote:
>> Static analysis reports that 'index' may be a user controlled value that
>> is used as a data dependency to read 'pin' from the
>> 'selector->baSourceID' array. In order to avoid potential leaks of
>> kernel memory values, block speculative execution of the instruction
>> stream that could issue reads based on an invalid value of 'pin'.
>
> I won't repeat the arguments already made in the thread regarding having
> documented coverity rules for this, even if I agree with them.
>
>> Based on an original patch by Elena Reshetova.
>>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> b/drivers/media/usb/uvc/uvc_v4l2.c index 3e7e283a44a8..7442626dc20e 100644
>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -22,6 +22,7 @@
>> #include <linux/mm.h>
>> #include <linux/wait.h>
>> #include <linux/atomic.h>
>> +#include <linux/compiler.h>
>>
>> #include <media/v4l2-common.h>
>> #include <media/v4l2-ctrls.h>
>> @@ -810,6 +811,7 @@ static int uvc_ioctl_enum_input(struct file *file, void
>> *fh, struct uvc_entity *iterm = NULL;
>> u32 index = input->index;
>> int pin = 0;
>> + __u8 *elem;
>>
>> if (selector == NULL ||
>> (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
>> @@ -820,8 +822,9 @@ static int uvc_ioctl_enum_input(struct file *file, void
>> *fh, break;
>> }
>> pin = iterm->id;
>> - } else if (index < selector->bNrInPins) {
>> - pin = selector->baSourceID[index];
>> + } else if ((elem = nospec_array_ptr(selector->baSourceID, index,
>> + selector->bNrInPins))) {
>> + pin = *elem;
>> list_for_each_entry(iterm, &chain->entities, chain) {
>> if (!UVC_ENTITY_IS_ITERM(iterm))
>> continue;
>
> (adding a bit more context)
>
>> if (iterm->id == pin)
>> break;
>> }
>> }
>>
>> if (iterm == NULL || iterm->id != pin)
>> return -EINVAL;
>>
>> memset(input, 0, sizeof(*input));
>> input->index = index;
>> strlcpy(input->name, iterm->name, sizeof(input->name));
>> if (UVC_ENTITY_TYPE(iterm) == UVC_ITT_CAMERA)
>> input->type = V4L2_INPUT_TYPE_CAMERA;
>
> So pin is used to search for an entry in the chain->entities list. Entries in
> that list are allocated separately through kmalloc and can thus end up in
> different cache lines, so I agree we have an issue. However, this is mitigated
> by the fact that typical UVC devices have a handful (sometimes up to a dozen)
> entities, so an attacker would only be able to read memory values that are
> equal to the entity IDs used by the device. Entity IDs can be freely allocated
> but typically count continuously from 0. It would take a specially-crafted UVC
> device to be able to read all memory.
>
> On the other hand, as this is nowhere close to being a fast path, I think we
> can close this potential hole as proposed in the patch. So,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Thanks Laurent!
> Will you merge the whole series in one go, or would you like me to take the
> patch in my tree ? In the latter case I'll wait until the nospec_array_ptr()
> gets merged in mainline.
I'll track it for now. Until the 'nospec_array_ptr()' discussion
resolves there won't be a stabilized commit-id for you to base a
branch.
^ permalink raw reply
* Re: [PATCH net-next 1/3] ethtool: Ensure new ring parameters are within bounds during SRINGPARAM
From: Jakub Kicinski @ 2018-01-09 2:23 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David S. Miller, netdev, Eran Ben Elisha, Eugenia Emantayev
In-Reply-To: <1515420026-11970-2-git-send-email-tariqt@mellanox.com>
On Mon, 8 Jan 2018 16:00:24 +0200, Tariq Toukan wrote:
> From: Eugenia Emantayev <eugenia@mellanox.com>
>
> Add a sanity check to ensure that all requested ring parameters
> are within bounds, which should reduce errors in driver implementation.
(y)
> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> ---
> net/core/ethtool.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 50a79203043b..9ea7cd52fde0 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1704,14 +1704,23 @@ static int ethtool_get_ringparam(struct net_device *dev, void __user *useraddr)
>
> static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
> {
> - struct ethtool_ringparam ringparam;
> + struct ethtool_ringparam ringparam, max = { .cmd = ETHTOOL_GRINGPARAM };
>
> - if (!dev->ethtool_ops->set_ringparam)
> + if (!dev->ethtool_ops->set_ringparam || !dev->ethtool_ops->get_ringparam)
> return -EOPNOTSUPP;
>
> if (copy_from_user(&ringparam, useraddr, sizeof(ringparam)))
> return -EFAULT;
>
> + dev->ethtool_ops->get_ringparam(dev, &max);
Perhaps check the return value here? It's pretty unlikely but
get_ringparam may fail.
> + /* ensure new ring parameters are within the maximums */
> + if (ringparam.rx_pending > max.rx_max_pending ||
> + ringparam.rx_mini_pending > max.rx_mini_max_pending ||
> + ringparam.rx_jumbo_pending > max.rx_jumbo_max_pending ||
> + ringparam.tx_pending > max.tx_max_pending)
> + return -EINVAL;
> +
> return dev->ethtool_ops->set_ringparam(dev, &ringparam);
> }
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: introduce BPF_JIT_ALWAYS_ON config
From: Alexei Starovoitov @ 2018-01-09 2:23 UTC (permalink / raw)
To: Jakub Kicinski, Daniel Borkmann
Cc: Alexei Starovoitov, davem, torvalds, jannh, alan, netdev,
kernel-team
In-Reply-To: <20180108160218.1e735fca@cakuba.netronome.com>
On 1/8/18 4:02 PM, Jakub Kicinski wrote:
> On Mon, 8 Jan 2018 22:59:04 +0100, Daniel Borkmann wrote:
>>> @@ -1453,6 +1457,11 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
>>> */
>>> *err = bpf_check_tail_call(fp);
>>>
>>> +#ifdef CONFIG_BPF_JIT_ALWAYS_ON
>>> + if (!fp->jited)
>>> + *err = -ENOTSUPP;
>>> +#endif
>
> I think programs JITed for offload won't have fp->jited set, but
> those are pretty safe from CPU bugs. Should we set fp->jited = 1; in
> bpf_prog_offload_compile()? Just throwing "&& !bpf_prog_is_dev_bound()"
> in here seems cleaner to me.
good catch. will fix in the v2.
^ permalink raw reply
* RE: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
From: Chris Mi @ 2018-01-09 2:35 UTC (permalink / raw)
To: Phil Sutter
Cc: dsahern@gmail.com, marcelo.leitner@gmail.com,
netdev@vger.kernel.org, gerlitz.or@gmail.com,
stephen@networkplumber.org
In-Reply-To: <20180108133150.GE14358@orbyte.nwl.cc>
> -----Original Message-----
> From: n0-1@orbyte.nwl.cc [mailto:n0-1@orbyte.nwl.cc] On Behalf Of Phil
> Sutter
> Sent: Monday, January 8, 2018 9:32 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: dsahern@gmail.com; marcelo.leitner@gmail.com;
> netdev@vger.kernel.org; gerlitz.or@gmail.com;
> stephen@networkplumber.org
> Subject: Re: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
>
> Hi Chris,
>
> On Mon, Jan 08, 2018 at 02:03:53AM +0000, Chris Mi wrote:
> > > On Thu, Jan 04, 2018 at 04:34:51PM +0900, Chris Mi wrote:
> > > > The insertion rate is improved more than 10%.
> > >
> > > Did you measure the effect of increasing batch sizes?
> > Yes. Even if we enlarge the batch size bigger than 10, there is no big
> improvement.
> > I think that's because current kernel doesn't process the requests in
> parallel.
> > If kernel processes the requests in parallel, I believe specifying a
> > bigger batch size will get a better result.
>
> But throughput doesn't regress at some point, right? I think that's the critical
> aspect when considering an "unlimited" batch size.
>
> On Mon, Jan 08, 2018 at 08:00:00AM +0000, Chris Mi wrote:
> > After testing, I find that the message passed to kernel should not be too
> big.
> > If it is bigger than about 64K, sendmsg returns -1, errno is 90 (EMSGSIZE).
> > That is about 400 commands. So how about set batch size to 128 which is
> big enough?
>
> If that's the easiest way, why not. At first, I thought one could maybe send
> the collected messages in chunks of suitable size, but that's probably not
> worth the effort.
I did a testing. If we read a million commands in memory and send them in chunks of 128,
we'll have a big regression. It takes about 21 seconds.
^ permalink raw reply
* RE: [PATCH net-next v2 5/6] bnx2x: Replace WARN_ONCE with netdev_WARN_ONCE
From: Kalluru, Sudarsana @ 2018-01-09 2:40 UTC (permalink / raw)
To: Gal Pressman, David S. Miller
Cc: netdev@vger.kernel.org, Tariq Toukan, Saeed Mahameed,
Elior, Ariel
In-Reply-To: <1515319720-18428-6-git-send-email-galp@mellanox.com>
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Gal Pressman
Sent: 07 January 2018 15:39
To: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org; Tariq Toukan <tariqt@mellanox.com>; Saeed Mahameed <saeedm@mellanox.com>; Gal Pressman <galp@mellanox.com>; Elior, Ariel <Ariel.Elior@cavium.com>
Subject: [PATCH net-next v2 5/6] bnx2x: Replace WARN_ONCE with netdev_WARN_ONCE
Use the more appropriate netdev_WARN_ONCE instead of WARN_ONCE macro.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
Cc: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 57eb26d..d7c98e8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -738,8 +738,9 @@ static void bnx2x_gro_receive(struct bnx2x *bp, struct bnx2x_fastpath *fp,
bnx2x_gro_csum(bp, skb, bnx2x_gro_ipv6_csum);
break;
default:
- WARN_ONCE(1, "Error: FW GRO supports only IPv4/IPv6, not 0x%04x\n",
- be16_to_cpu(skb->protocol));
+ netdev_WARN_ONCE(bp->dev,
+ "Error: FW GRO supports only IPv4/IPv6, not 0x%04x\n",
+ be16_to_cpu(skb->protocol));
}
}
#endif
--
2.7.4
Acked-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>
^ permalink raw reply related
* Re: [PATCH net-next v2 1/6] net: Fix netdev_WARN_ONCE macro
From: Joe Perches @ 2018-01-09 2:42 UTC (permalink / raw)
To: Gal Pressman, David S. Miller; +Cc: netdev, Tariq Toukan, Saeed Mahameed
In-Reply-To: <1515319720-18428-2-git-send-email-galp@mellanox.com>
On Sun, 2018-01-07 at 12:08 +0200, Gal Pressman wrote:
> netdev_WARN_ONCE is broken (whoops..), this fix will remove the
> unnecessary "condition" parameter, add the missing comma and change
> "arg" to "args".
>
> Fixes: 375ef2b1f0d0 ("net: Introduce netdev_*_once functions")
> Signed-off-by: Gal Pressman <galp@mellanox.com>
> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> include/linux/netdevice.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 352066e..5ff1ef9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4407,8 +4407,8 @@ do { \
> WARN(1, "netdevice: %s%s\n" format, netdev_name(dev), \
> netdev_reg_state(dev), ##args)
>
> -#define netdev_WARN_ONCE(dev, condition, format, arg...) \
> - WARN_ONCE(1, "netdevice: %s%s\n" format, netdev_name(dev) \
> +#define netdev_WARN_ONCE(dev, format, args...) \
> + WARN_ONCE(1, "netdevice: %s%s\n" format, netdev_name(dev), \
You sure you want the newline before the format?
> netdev_reg_state(dev), ##args)
>
> /* netif printk helpers, similar to netdev_printk */
^ permalink raw reply
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: lipeng (Y) @ 2018-01-09 2:48 UTC (permalink / raw)
To: David Miller, kubakici; +Cc: netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108.205440.1825662768273316692.davem@davemloft.net>
On 2018/1/9 9:54, David Miller wrote:
> From: Jakub Kicinski <kubakici@wp.pl>
> Date: Mon, 8 Jan 2018 17:50:21 -0800
>
>> Oh, I only noticed this extra misleading comment now. Unless each queue
>> has a netdev, I don't see how these are per-queue.
> If it isn't per-queue I want this change reverted.
[patch 12/20 ] add statistics of netdev for ethtool -S, netdev may have
multi queue.
As discussion here, it is duplicate to add this patch.
I revert [patch 12/20 ] , and then test on my board, HNS3 basic function and ethtool -S work well.
So I think it is OK if you can revert [patch 12/20 ]("net: hns3: Add packet statistics of netdev").
Thanks
Peng Li
> .
>
^ permalink raw reply
* [v2] net: gianfar_ptp: move set_fipers() to spinlock protecting area
From: Yangbo Lu @ 2018-01-09 3:02 UTC (permalink / raw)
To: Fabio Estevam, Claudiu Manoil, Richard Cochran, netdev,
linux-kernel
Cc: Yangbo Lu
set_fipers() calling should be protected by spinlock in
case that any interrupt breaks related registers setting
and the function we expect. This patch is to move set_fipers()
to spinlock protecting area in ptp_gianfar_adjtime().
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- explained why spinlock was needed in commit message.
---
drivers/net/ethernet/freescale/gianfar_ptp.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index 5441142..9f8d4f8 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -319,11 +319,10 @@ static int ptp_gianfar_adjtime(struct ptp_clock_info *ptp, s64 delta)
now = tmr_cnt_read(etsects);
now += delta;
tmr_cnt_write(etsects, now);
+ set_fipers(etsects);
spin_unlock_irqrestore(&etsects->lock, flags);
- set_fipers(etsects);
-
return 0;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing
From: Ed Swierk @ 2018-01-09 3:02 UTC (permalink / raw)
To: Pravin Shelar; +Cc: ovs-dev, netdev, Benjamin Warren, Keith Holleman
In-Reply-To: <CAOrHB_A1sQWforWuUve5phrxPjSOpbTEQ4F5h3beh1C9qWmUTw@mail.gmail.com>
On 1/6/18 10:57, Pravin Shelar wrote:
> On Fri, Jan 5, 2018 at 10:59 PM, Ed Swierk <eswierk@skyportsystems.com> wrote:
>>
>>
>> On Jan 5, 2018 22:17, "Pravin Shelar" <pshelar@ovn.org> wrote:
>>
>> On Fri, Jan 5, 2018 at 3:20 PM, Ed Swierk <eswierk@skyportsystems.com>
>> wrote:
>>> On Fri, Jan 5, 2018 at 10:14 AM, Ed Swierk <eswierk@skyportsystems.com>
>>> wrote:
>>>> On Thu, Jan 4, 2018 at 7:36 PM, Pravin Shelar <pshelar@ovn.org> wrote:
>>>>> OVS already pull all required headers in skb linear data, so no need
>>>>> to redo all of it. only check required is the ip-checksum validation.
>>>>> I think we could avoid it in most of cases by checking skb length to
>>>>> ipheader length before verifying the ip header-checksum.
>>>>
>>>> Shouldn't the IP header checksum be verified even earlier, like in
>>>> key_extract(), before actually using any of the fields in the IP
>>>> header?
>>>
>>> Something like this for verifying the IP header checksum (not tested):
>>>
>> AFAIU openflow does not need this verification, so it is not required
>> in flow extract.
>>
>>
>> Okay. How about my proposed trimming implementation, caching the pad length
>> in the ovs cb?
>>
> Caching the length is not that simple, OVS actions can change the
> length. Keeping it consistent with packet would be more work, so lets
> calculate it in ovs-ct function.
>
Something like this?
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a38c80e..282325d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4084,6 +4084,8 @@ struct sk_buff *skb_checksum_trimmed(struct sk_buff *skb,
unsigned int transport_len,
__sum16(*skb_chkf)(struct sk_buff *skb));
+int skb_network_trim(struct sk_buff *skb);
+
/**
* skb_head_is_locked - Determine if the skb->head is locked down
* @skb: skb to check
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 08f5740..c68e927 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4740,6 +4740,41 @@ struct sk_buff *skb_checksum_trimmed(struct sk_buff *skb,
}
EXPORT_SYMBOL(skb_checksum_trimmed);
+/**
+ * skb_network_trim - trim skb to length specified by the network header
+ * @skb: the skb to trim
+ *
+ * Trims the skb to the length specified by the network header,
+ * removing any trailing padding. Leaves the skb alone if the protocol
+ * is not IP or IPv6. Frees the skb on error.
+ *
+ * Caller needs to pull the skb to the network header.
+ */
+int skb_network_trim(struct sk_buff *skb)
+{
+ unsigned int len;
+ int err;
+
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ len = ntohs(ip_hdr(skb)->tot_len);
+ break;
+ case htons(ETH_P_IPV6):
+ len = sizeof(struct ipv6hdr)
+ + ntohs(ipv6_hdr(skb)->payload_len);
+ break;
+ default:
+ len = skb->len;
+ }
+
+ err = pskb_trim_rcsum(skb, len);
+ if (unlikely(err))
+ kfree_skb(skb);
+
+ return err;
+}
+EXPORT_SYMBOL(skb_network_trim);
+
void __skb_warn_lro_forwarding(const struct sk_buff *skb)
{
net_warn_ratelimited("%s: received packets cannot be forwarded while LRO is enabled\n",
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index b27c5c6..73418d3 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1112,6 +1112,10 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
nh_ofs = skb_network_offset(skb);
skb_pull_rcsum(skb, nh_ofs);
+ err = skb_network_trim(skb);
+ if (err)
+ return err;
+
if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
err = handle_fragments(net, key, info->zone.id, skb);
if (err)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox