* Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Neftin, Sasha @ 2017-08-29 7:19 UTC (permalink / raw)
To: Matthew Tan, jeffrey.t.kirsher
Cc: michael.kardonik, mitch.a.williams, linux-kernel, john.ronciak,
intel-wired-lan, netdev
In-Reply-To: <1503503985-3869-1-git-send-email-matthew.tan_1@nxp.com>
On 8/23/2017 18:59, Matthew Tan wrote:
> Calls to udelay are not preemtable by userspace so userspace
> applications experience a large (~200us) latency when running on core
> 0. Instead usleep_range can be used to be more friendly to userspace
> since it is preemtable. This is due to udelay using busy-wait loops
> while usleep_rang uses hrtimers instead. It is recommended to use
> udelay when the delay is <10us since at that precision overhead of
> usleep_range hrtimer setup causes issues. However, the replaced calls
> are for 50us and 100us so this should not be not an issue.
>
> Signed-off-by: Matthew Tan <matthew.tan_1@nxp.com>
> ---
> drivers/net/ethernet/intel/e1000e/phy.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
> index de13aea..e318fdc 100644
> --- a/drivers/net/ethernet/intel/e1000e/phy.c
> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
> @@ -158,7 +158,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
> * the lower time out
> */
> for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
> - udelay(50);
> + usleep_range(40, 60);
> mdic = er32(MDIC);
> if (mdic & E1000_MDIC_READY)
> break;
> @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
> * reading duplicate data in the next MDIC transaction.
> */
> if (hw->mac.type == e1000_pch2lan)
> - udelay(100);
> + usleep_range(90, 100);
>
> return 0;
> }
> @@ -222,7 +222,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
> * the lower time out
> */
> for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
> - udelay(50);
> + usleep_range(40, 60);
> mdic = er32(MDIC);
> if (mdic & E1000_MDIC_READY)
> break;
> @@ -246,7 +246,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
> * reading duplicate data in the next MDIC transaction.
> */
> if (hw->mac.type == e1000_pch2lan)
> - udelay(100);
> + usleep_range(90, 110);
>
> return 0;
> }
Reasonable. Do you have any open bug or other reference describe this
problem?
^ permalink raw reply
* Re: [patch net-next 1/3] idr: Add new APIs to support unsigned long
From: Hannes Frederic Sowa @ 2017-08-29 7:14 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, mawilcox
In-Reply-To: <1503902477-39829-2-git-send-email-chrism@mellanox.com>
Hello,
Chris Mi <chrism@mellanox.com> writes:
> The following new APIs are added:
>
> int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> unsigned long start, unsigned long end, gfp_t gfp);
> static inline void *idr_remove_ext(struct idr *idr, unsigned long id);
> static inline void *idr_find_ext(const struct idr *idr, unsigned long id);
> void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
> void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/idr.h | 16 ++++++++++
> include/linux/radix-tree.h | 3 ++
> lib/idr.c | 56 +++++++++++++++++++++++++++++++++++
> lib/radix-tree.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 148 insertions(+)
>
[...]
> +int idr_alloc_ext(struct idr *idr, void *ptr, unsigned long *index,
> + unsigned long start, unsigned long end, gfp_t gfp)
> +{
> + void __rcu **slot;
> + struct radix_tree_iter iter;
> +
> + if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
> + return -EINVAL;
> +
> + radix_tree_iter_init(&iter, start);
> + slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
> + if (IS_ERR(slot))
> + return PTR_ERR(slot);
> +
> + radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
> + radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
> +
> + if (index)
> + *index = iter.index;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(idr_alloc_ext);
Can you express idr_alloc in terms of idr_alloc_ext? Same for most of
the other functions (it seems that signed int was used as return value
to indicate error cases, thus it should be easy to map those).
[...]
Thanks,
Hannes
^ permalink raw reply
* [PATCH net-next] staging: irda: update MAINTAINERS
From: Greg Kroah-Hartman @ 2017-08-29 7:09 UTC (permalink / raw)
To: davem, samuel; +Cc: devel, netdev, Joe Perches, linux-kernel
Now that the IRDA code has moved under drivers/staging/irda/, update the
MAINTAINERS file with the new location.
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
---
MAINTAINERS | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6fdfe2685eed..ff19b1c3141c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7101,9 +7101,7 @@ W: http://irda.sourceforge.net/
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/sameo/irda-2.6.git
F: Documentation/networking/irda.txt
-F: drivers/net/irda/
-F: include/net/irda/
-F: net/irda/
+F: drivers/staging/irda/
IRQ DOMAINS (IRQ NUMBER MAPPING LIBRARY)
M: Marc Zyngier <marc.zyngier@arm.com>
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next] Revert "ipv4: make net_protocol const"
From: Bhumika Goyal @ 2017-08-29 6:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, David Miller
In-Reply-To: <1503951789-31836-1-git-send-email-dsahern@gmail.com>
On Tue, Aug 29, 2017 at 1:53 AM, David Ahern <dsahern@gmail.com> wrote:
> This reverts commit aa8db499ea67cff1f5f049033810ffede2fe5ae4.
>
> Early demux structs can not be made const. Doing so results in:
> [ 84.967355] BUG: unable to handle kernel paging request at ffffffff81684b10
> [ 84.969272] IP: proc_configure_early_demux+0x1e/0x3d
> [ 84.970544] PGD 1a0a067
> [ 84.970546] P4D 1a0a067
> [ 84.971212] PUD 1a0b063
> [ 84.971733] PMD 80000000016001e1
>
> [ 84.972669] Oops: 0003 [#1] SMP
> [ 84.973065] Modules linked in: ip6table_filter ip6_tables veth vrf
> [ 84.973833] CPU: 0 PID: 955 Comm: sysctl Not tainted 4.13.0-rc6+ #22
> [ 84.974612] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 84.975855] task: ffff88003854ce00 task.stack: ffffc900005a4000
> [ 84.976580] RIP: 0010:proc_configure_early_demux+0x1e/0x3d
> [ 84.977253] RSP: 0018:ffffc900005a7dd0 EFLAGS: 00010246
> [ 84.977891] RAX: ffffffff81684b10 RBX: 0000000000000001 RCX: 0000000000000000
> [ 84.978759] RDX: 0000000000000000 RSI: 0000000000000006 RDI: 0000000000000000
> [ 84.979628] RBP: ffffc900005a7dd0 R08: 0000000000000000 R09: 0000000000000000
> [ 84.980501] R10: 0000000000000001 R11: 0000000000000008 R12: 0000000000000001
> [ 84.981373] R13: ffffffffffffffea R14: ffffffff81a9b4c0 R15: 0000000000000002
> [ 84.982249] FS: 00007feb237b7700(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
> [ 84.983231] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 84.983941] CR2: ffffffff81684b10 CR3: 0000000038492000 CR4: 00000000000406f0
> [ 84.984817] Call Trace:
> [ 84.985133] proc_tcp_early_demux+0x29/0x30
>
> I think this is the second time such a patch has been reverted.
>
> Cc: Bhumika Goyal <bhumirks@gmail.com>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> Bhumika: How are you testing these constify changes? In this case a simple
> sysctl -w net.ipv4.tcp_early_demux=1 would have shown the problem
>
I am compile testing them. In this case I did: make
net/ipv4/af_inet.o and it compiled. Is this error because of
typecasting net_protocol inside inet_add_protocol function?
Thanks,
Bhumika
> net/ipv4/af_inet.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 19aee073ba29..d678820e4306 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1596,7 +1596,7 @@ static const struct net_protocol igmp_protocol = {
> };
> #endif
>
> -static const struct net_protocol tcp_protocol = {
> +static struct net_protocol tcp_protocol = {
> .early_demux = tcp_v4_early_demux,
> .early_demux_handler = tcp_v4_early_demux,
> .handler = tcp_v4_rcv,
> @@ -1606,7 +1606,7 @@ static const struct net_protocol tcp_protocol = {
> .icmp_strict_tag_validation = 1,
> };
>
> -static const struct net_protocol udp_protocol = {
> +static struct net_protocol udp_protocol = {
> .early_demux = udp_v4_early_demux,
> .early_demux_handler = udp_v4_early_demux,
> .handler = udp_rcv,
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: dsa: add generic debugfs interface
From: Jiri Pirko @ 2017-08-29 6:29 UTC (permalink / raw)
To: David Miller
Cc: vivien.didelot, netdev, linux-kernel, kernel, f.fainelli, andrew,
privat, john, Woojung.Huh, sean.wang, nikita.yoush, cphealy
In-Reply-To: <20170828.213837.1354872205076475221.davem@davemloft.net>
Tue, Aug 29, 2017 at 06:38:37AM CEST, davem@davemloft.net wrote:
>From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>Date: Mon, 28 Aug 2017 15:17:38 -0400
>
>> This patch series adds a generic debugfs interface for the DSA
>> framework, so that all switch devices benefit from it, e.g. Marvell,
>> Broadcom, Microchip or any other DSA driver.
>
>I've been thinking this over and I agree with the feedback given that
>debugfs really isn't appropriate for this.
>
>Please create a DSA device class, and hang these values under
>appropriate sysfs device nodes that can be easily found via
>/sys/class/dsa/ just as easily as they would be /sys/kernel/debug/dsa/
>
>You really intend these values to be consistent across DSA devices,
>and you don't intend to go willy-nilly changig these exported values
>arbitrarily over time. That's what debugfs is for, throw-away
>stuff.
>
>So please make these proper device sysfs attributes rather than
>debugfs.
As I wrote, I believe that there is a big overlap with devlink and its
dpipe subset. I think that primary we should focus on extending whatever
is needed for dsa there. The iface should be generic for all drivers,
not only dsa. dsa-specific sysfs attributes should be last-resort solution,
I believe we can avoid them.
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: dsa: add generic debugfs interface
From: Jiri Pirko @ 2017-08-29 6:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, netdev, linux-kernel, kernel, David S. Miller,
Florian Fainelli, Egil Hjelmeland, John Crispin, Woojung Huh,
Sean Wang, Nikita Yushchenko, Chris Healy, mlxsw
In-Reply-To: <20170828200834.GA1870@lunn.ch>
Mon, Aug 28, 2017 at 10:08:34PM CEST, andrew@lunn.ch wrote:
>> I see this overlaps a lot with DPIPE. Why won't you use that to expose
>> your hw state?
>
>We took a look at dpipe and i talked to you about using it for this
>sort of thing at netconf/netdev. But dpipe has issues displaying the
>sort of information we have. I never figured out how to do two
>dimensional tables. The output of the dpipe command is pretty
>unreadable. A lot of the information being dumped here is not about
>the data pipe, etc.
So improve it. No problem. Also, we extend it to support what you neede.
>
>There is a lot of pushback on debugfs for individual drivers. As i
>said recently to somebody, debugfs is a bit of a wild west. When
>designing this code, we thought about that. This debugfs is not at the
>driver level. It is at the DSA level. All DSA drivers will benefit
>from this code, and all DSA drivers will get the same information
>exposed in debugfs. It is generic, well defined and structured, with
>respect to DSA.
Still, it has *a lot* of overlap with devlink and dpipe. So instead of
making devlink and dpipe work for you, you introduced completely
separated debugfs interface specific to a list of drivers. That is just
wrong. Debugfs is never the correct answer! Please work with us on
devlink and dpipe so they are used for all drivers, mlxsw, dsa and others.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next] bnxt_en: add a dummy definition for bnxt_vf_rep_get_fid()
From: Michael Chan @ 2017-08-29 6:22 UTC (permalink / raw)
To: Sathya Perla, David Miller; +Cc: Netdev
In-Reply-To: <1503987303-12392-1-git-send-email-sathya.perla@broadcom.com>
On Mon, Aug 28, 2017 at 11:15 PM, Sathya Perla
<sathya.perla@broadcom.com> wrote:
> When bnxt VF-reps are not compiled in (CONFIG_BNXT_SRIOV is off)
> bnxt_tc.c needs a dummy definition of the routine bnxt_vf_rep_get_fid().
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Fixes: 2ae7408fedfe ("bnxt_en: bnxt: add TC flower filter offload support")
> Signed-off-by: Sathya Perla <sathya.perla@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* [PATCH net-next] bnxt_en: add a dummy definition for bnxt_vf_rep_get_fid()
From: Sathya Perla @ 2017-08-29 6:15 UTC (permalink / raw)
To: netdev
When bnxt VF-reps are not compiled in (CONFIG_BNXT_SRIOV is off)
bnxt_tc.c needs a dummy definition of the routine bnxt_vf_rep_get_fid().
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Fixes: 2ae7408fedfe ("bnxt_en: bnxt: add TC flower filter offload support")
Signed-off-by: Sathya Perla <sathya.perla@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
index d8b5f89..7787cd24 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h
@@ -80,5 +80,10 @@ static inline struct net_device *bnxt_get_vf_rep(struct bnxt *bp, u16 cfa_code)
{
return NULL;
}
+
+static inline u16 bnxt_vf_rep_get_fid(struct net_device *dev)
+{
+ return 0;
+}
#endif /* CONFIG_BNXT_SRIOV */
#endif /* BNXT_VFR_H */
--
2.7.4
^ permalink raw reply related
* Re: mlxsw and rtnl lock
From: Arkadi Sharshevsky @ 2017-08-29 6:10 UTC (permalink / raw)
To: David Ahern, Ido Schimmel; +Cc: Jiri Pirko, netdev@vger.kernel.org, mlxsw
In-Reply-To: <b6dc0a4e-4ed9-ed5f-ac0f-c3fe06d5ed68@gmail.com>
On 08/28/2017 09:00 PM, David Ahern wrote:
> On 8/26/17 11:04 AM, Ido Schimmel wrote:
>> Regarding the silent abort, that's intentional. You can look at the same
>> code in v4.9 - when the chain was still blocking - and you'll see that
>> we didn't propagate the error even then. This was discussed in the past
>> and the conclusion was that user doesn't expect to operation to fail. If
>> hardware resources are exceeded, we let the kernel take care of the
>> forwarding instead.
>>
>
> In addition to Roopa's comments... The silent abort is not a good user
> experience. Right now it's add a network address or route, cross fingers
> and hope it does not overflow some limit (nexthop, ecmp, neighbor,
> prefix, etc) that triggers the offload abort.
>
> The mlxsw driver queries for some limits (e.g., max rifs) but I don't
> see any query related to current usage, and there is no API to pass any
> of that data to user space so user space has no programmatic way to
> handle this. I realize you are aware of this limitation. The point is to
> emphasize the need to resolve this.
>
We actually thought about providing he user some tools to understand
the ASIC's limitations by introducing the 'resource' object to devlink.
By linking dpipe tables to resources the user can understand which
hardware processes share a common resource, furthermore this resources
usage could be observed. By this more visibility can be obtained.
Its not a remedy for the silent abort, but, maybe a notification
can be sent from devlink in case of abort that some resources is
full.
This proposition was sent as RFC several weeks ago.
^ permalink raw reply
* (unknown),
From: morice.diane @ 2017-08-29 5:40 UTC (permalink / raw)
To: netdev
[-- Attachment #1: MAIL_81389397283742_netdev.zip --]
[-- Type: application/zip, Size: 72397 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] irda: move it to drivers/staging so we can delete it
From: Greg KH @ 2017-08-29 5:06 UTC (permalink / raw)
To: Joe Perches; +Cc: devel, netdev, samuel, David Miller, linux-kernel
In-Reply-To: <1503963967.2040.14.camel@perches.com>
On Mon, Aug 28, 2017 at 04:46:07PM -0700, Joe Perches wrote:
> On Mon, 2017-08-28 at 16:42 -0700, David Miller wrote:
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Date: Sun, 27 Aug 2017 17:03:30 +0200
> >
> > > The IRDA code has long been obsolete and broken. So, to keep people
> > > from trying to use it, and to prevent people from having to maintain it,
> > > let's move it to drivers/staging/ so that we can delete it entirely from
> > > the kernel in a few releases.
> >
> > No objection, I'll apply this to net-next, thanks Greg.
>
> Still needs an update to MAINTAINERS.
Oops, forgot those directories, will send a follow-on patch for that.
greg k-h
^ permalink raw reply
* Re: [PATCH net 0/4] xfrm_user info leaks
From: Steffen Klassert @ 2017-08-29 4:43 UTC (permalink / raw)
To: David Miller; +Cc: minipli, herbert, netdev
In-Reply-To: <20170828.155232.1540318133719787999.davem@davemloft.net>
On Mon, Aug 28, 2017 at 03:52:32PM -0700, David Miller wrote:
> From: Mathias Krause <minipli@googlemail.com>
> Date: Sat, 26 Aug 2017 17:08:56 +0200
>
> > Hi David, Steffen,
> >
> > the following series fixes a few info leaks due to missing padding byte
> > initialization in the xfrm_user netlink interface.
> >
> > Please apply!
>
> Steffen please pick this up if you haven't already.
I had it already in the ipsec/testing branch, now merged into
ipsec/master.
Thanks everyone!
^ permalink raw reply
* Re: [PATCH net-next] hinic: don't build the module by default
From: David Miller @ 2017-08-29 4:40 UTC (permalink / raw)
To: vkuznets; +Cc: netdev, aviad.krawczyk, linux-kernel
In-Reply-To: <20170828131605.3173-1-vkuznets@redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Mon, 28 Aug 2017 15:16:05 +0200
> We probably don't want to enable code supporting particular hardware by
> default e.g. when someone does 'make defconfig'. Other ethernet modules
> don't do it.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: dsa: add generic debugfs interface
From: David Miller @ 2017-08-29 4:38 UTC (permalink / raw)
To: vivien.didelot
Cc: netdev, linux-kernel, kernel, f.fainelli, andrew, privat, john,
Woojung.Huh, sean.wang, nikita.yoush, cphealy
In-Reply-To: <20170828191748.19492-1-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Mon, 28 Aug 2017 15:17:38 -0400
> This patch series adds a generic debugfs interface for the DSA
> framework, so that all switch devices benefit from it, e.g. Marvell,
> Broadcom, Microchip or any other DSA driver.
I've been thinking this over and I agree with the feedback given that
debugfs really isn't appropriate for this.
Please create a DSA device class, and hang these values under
appropriate sysfs device nodes that can be easily found via
/sys/class/dsa/ just as easily as they would be /sys/kernel/debug/dsa/
You really intend these values to be consistent across DSA devices,
and you don't intend to go willy-nilly changig these exported values
arbitrarily over time. That's what debugfs is for, throw-away
stuff.
So please make these proper device sysfs attributes rather than
debugfs.
Thank you.
^ permalink raw reply
* Re: [net-next] be2net: use shift instead of expensive divide
From: David Miller @ 2017-08-29 4:25 UTC (permalink / raw)
To: zhangshengju; +Cc: sathya.perla, ajit.khaparde, sriharsha.basavapatna, netdev
In-Reply-To: <1503976719-3063-1-git-send-email-zhangshengju@cmss.chinamobile.com>
From: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Date: Tue, 29 Aug 2017 11:18:39 +0800
> Replace shift instead of expensive divide.
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
The divide is more easier to understand, and it costs the same
as a shift if the types are unsigned.
I'm not applying silly changes like this which actually make
the code worse off, sorry.
^ permalink raw reply
* [PATCH net-next v2] bridge: fdb add and delete tracepoints
From: Roopa Prabhu @ 2017-08-29 4:22 UTC (permalink / raw)
To: davem; +Cc: netdev, nikolay, f.fainelli, bridge
From: Roopa Prabhu <roopa@cumulusnetworks.com>
A few useful tracepoints to trace bridge forwarding
database updates.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
v2 - address comments from Florian
include/trace/events/bridge.h | 98 +++++++++++++++++++++++++++++++++++++++++
net/bridge/br_fdb.c | 7 +++
net/core/net-traces.c | 6 +++
3 files changed, 111 insertions(+)
create mode 100644 include/trace/events/bridge.h
diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
new file mode 100644
index 0000000..3a4ecc3
--- /dev/null
+++ b/include/trace/events/bridge.h
@@ -0,0 +1,98 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM bridge
+
+#if !defined(_TRACE_BRIDGE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_BRIDGE_H
+
+#include <linux/netdevice.h>
+#include <linux/tracepoint.h>
+
+#include "../../../net/bridge/br_private.h"
+
+TRACE_EVENT(br_fdb_add,
+
+ TP_PROTO(struct ndmsg *ndm, struct net_device *dev,
+ const unsigned char *addr, u16 vid, u16 nlh_flags),
+
+ TP_ARGS(ndm, dev, addr, vid, nlh_flags),
+
+ TP_STRUCT__entry(
+ __field(u8, ndm_flags)
+ __string(dev, dev->name)
+ __array(unsigned char, addr, ETH_ALEN)
+ __field(u16, vid)
+ __field(u16, nlh_flags)
+ ),
+
+ TP_fast_assign(
+ __assign_str(dev, dev->name);
+ memcpy(__entry->addr, addr, ETH_ALEN);
+ __entry->vid = vid;
+ __entry->nlh_flags = nlh_flags;
+ __entry->ndm_flags = ndm->ndm_flags;
+ ),
+
+ TP_printk("dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u nlh_flags %04x ndm_flags = %02x",
+ __get_str(dev), __entry->addr[0], __entry->addr[1],
+ __entry->addr[2], __entry->addr[3], __entry->addr[4],
+ __entry->addr[5], __entry->vid,
+ __entry->nlh_flags, __entry->ndm_flags)
+);
+
+TRACE_EVENT(br_fdb_external_learn_add,
+
+ TP_PROTO(struct net_bridge *br, struct net_bridge_port *p,
+ const unsigned char *addr, u16 vid),
+
+ TP_ARGS(br, p, addr, vid),
+
+ TP_STRUCT__entry(
+ __string(br_dev, br->dev->name)
+ __string(dev, p->dev->name)
+ __array(unsigned char, addr, ETH_ALEN)
+ __field(u16, vid)
+ ),
+
+ TP_fast_assign(
+ __assign_str(br_dev, br ? br->dev->name : "null");
+ __assign_str(dev, p ? p->dev->name : "null");
+ memcpy(__entry->addr, addr, ETH_ALEN);
+ __entry->vid = vid;
+ ),
+
+ TP_printk("br_dev %s port %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u",
+ __get_str(br_dev), __get_str(dev), __entry->addr[0],
+ __entry->addr[1], __entry->addr[2], __entry->addr[3],
+ __entry->addr[4], __entry->addr[5], __entry->vid)
+);
+
+TRACE_EVENT(fdb_delete,
+
+ TP_PROTO(struct net_bridge *br, struct net_bridge_fdb_entry *f),
+
+ TP_ARGS(br, f),
+
+ TP_STRUCT__entry(
+ __string(br_dev, br->dev->name)
+ __string(dev, f->dst ? f->dst->dev->name : "null")
+ __array(unsigned char, addr, ETH_ALEN)
+ __field(u16, vid)
+ ),
+
+ TP_fast_assign(
+ __assign_str(br_dev, br ? br->dev->name : "null");
+ __assign_str(dev, f->dst ? f->dst->dev->name : "null");
+ memcpy(__entry->addr, f->addr.addr, ETH_ALEN);
+ __entry->vid = f->vlan_id;
+ ),
+
+ TP_printk("br_dev %s dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u",
+ __get_str(br_dev), __get_str(dev), __entry->addr[0],
+ __entry->addr[1], __entry->addr[2], __entry->addr[3],
+ __entry->addr[4], __entry->addr[5], __entry->vid)
+);
+
+#endif /* _TRACE_BRIDGE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index a79b648..be5e1da 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -25,6 +25,7 @@
#include <asm/unaligned.h>
#include <linux/if_vlan.h>
#include <net/switchdev.h>
+#include <trace/events/bridge.h>
#include "br_private.h"
static struct kmem_cache *br_fdb_cache __read_mostly;
@@ -171,6 +172,8 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
{
+ trace_fdb_delete(br, f);
+
if (f->is_static)
fdb_del_hw_addr(br, f->addr.addr);
@@ -870,6 +873,8 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_bridge *br = NULL;
int err = 0;
+ trace_br_fdb_add(ndm, dev, addr, vid, nlh_flags);
+
if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n", ndm->ndm_state);
return -EINVAL;
@@ -1066,6 +1071,8 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
bool modified = false;
int err = 0;
+ trace_br_fdb_external_learn_add(br, p, addr, vid);
+
spin_lock_bh(&br->hash_lock);
head = &br->hash[br_mac_hash(addr, vid)];
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index 4f1468c..4a0292c 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -37,6 +37,12 @@
#include <trace/events/fib6.h>
EXPORT_TRACEPOINT_SYMBOL_GPL(fib6_table_lookup);
#endif
+#if IS_ENABLED(CONFIG_BRIDGE)
+#include <trace/events/bridge.h>
+EXPORT_TRACEPOINT_SYMBOL_GPL(br_fdb_add);
+EXPORT_TRACEPOINT_SYMBOL_GPL(br_fdb_external_learn_add);
+EXPORT_TRACEPOINT_SYMBOL_GPL(fdb_delete);
+#endif
EXPORT_TRACEPOINT_SYMBOL_GPL(kfree_skb);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] be2net: Fix some u16 fields appropriately
From: David Miller @ 2017-08-29 4:22 UTC (permalink / raw)
To: yanhaishuang
Cc: sathya.perla, ajit.khaparde, sriharsha.basavapatna, somnath.kotur,
netdev, linux-kernel
In-Reply-To: <979AB2E6-7D90-4C6B-B43F-1D17EE621D05@cmss.chinamobile.com>
From: 严海双 <yanhaishuang@cmss.chinamobile.com>
Date: Tue, 29 Aug 2017 09:04:57 +0800
> The GET_TX_COMPL_BITS comes from amap_get which also returns a 32-bit value:
It never returns a value with more than 16-bits of significance for
this specific call.
Please stop trying to be semantically clever when arguing about this
change.
It's not about types, it's about what range of values the struct
member can actually hold.
^ permalink raw reply
* Re: [PATCH net] net: dsa: Don't dereference dst->cpu_dp->netdev
From: David Miller @ 2017-08-29 4:20 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, dan.carpenter
In-Reply-To: <1503965451-59498-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 28 Aug 2017 17:10:51 -0700
> If we do not have a master network device attached dst->cpu_dp will be
> NULL and accessing cpu_dp->netdev will create a trace similar to the one
> below. The correct check is on dst->cpu_dp period.
...
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: 6d3c8c0dd88a ("net: dsa: Remove master_netdev and use dst->cpu_dp->netdev")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks Florian.
^ permalink raw reply
* Re: net.ipv4.tcp_max_syn_backlog implementation
From: Eric Dumazet @ 2017-08-29 4:17 UTC (permalink / raw)
To: Harsha Chenji; +Cc: netdev
In-Reply-To: <CAMB9Wx+q9fidTnh3Tyias8KKdbRcKpoXM-ntSqjE_oW36F1W_A@mail.gmail.com>
On Mon, 2017-08-28 at 23:47 -0400, Harsha Chenji wrote:
> So I have ubuntu 12.04 x32 in a VM with syncookies turned off. I tried
> to do a syn flood (with netwox) on 3 different processes. Each of them
> returns a different value with netstat -na | grep -c RECV :
>
> nc -l 5555 returns 16 (netcat-traditional)
> apache2 port 80 returns 256
> vsftpd on 21 returns 64.
> net.ipv4.tcp_max_syn_backlog is 512.
>
> Why do these different processes on different ports have different
> queue lengths for incomplete connections? Where exactly in the kernel
> is this decided?
See 2nd argument in listen() system call, ie backlog
man listen
Without a synflood, just look at "ss -t state listening"
The backlog is the 2nd column (Send)
^ permalink raw reply
* Re: net.ipv4.tcp_max_syn_backlog implementation
From: Willy Tarreau @ 2017-08-29 4:12 UTC (permalink / raw)
To: Harsha Chenji; +Cc: netdev
In-Reply-To: <CAMB9Wx+q9fidTnh3Tyias8KKdbRcKpoXM-ntSqjE_oW36F1W_A@mail.gmail.com>
On Mon, Aug 28, 2017 at 11:47:41PM -0400, Harsha Chenji wrote:
> So I have ubuntu 12.04 x32 in a VM with syncookies turned off. I tried
> to do a syn flood (with netwox) on 3 different processes. Each of them
> returns a different value with netstat -na | grep -c RECV :
>
> nc -l 5555 returns 16 (netcat-traditional)
> apache2 port 80 returns 256
> vsftpd on 21 returns 64.
> net.ipv4.tcp_max_syn_backlog is 512.
>
> Why do these different processes on different ports have different
> queue lengths for incomplete connections? Where exactly in the kernel
> is this decided?
The listening socket's backlog (second argument to the listen() syscall)
is considered as well. The code path to determine whether or not to start
to send SYN cookies is far from being trivial but makes sense once you
write it down completely. I never perfectly remember it, I regularly have
to recheck when I have a doubt.
Willy
^ permalink raw reply
* Re: [PATCH v2 net-next 1/8] bpf: Add support for recursively running cgroup sock filters
From: Alexei Starovoitov @ 2017-08-29 4:11 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, daniel, ast, tj, davem, luto
In-Reply-To: <32706501-5fc3-7f59-9210-1898e896d384@gmail.com>
On Mon, Aug 28, 2017 at 08:22:31PM -0600, David Ahern wrote:
> On 8/28/17 7:12 PM, Alexei Starovoitov wrote:
> >>>> To consider what happens on doubling back and changing programs in the
> >>>> hierarchy, start with $MNT/a/b/c from 3 above (non-recursive on 'a',
> >>>> recursive on 'b' and recursive on 'c') for each of the following cases:
> >>>>
> >>>> 1. Program attached to 'b' is detached, recursive flag is reset in the
> >>>> request. Attempt fails EINVAL because the recursion flag has to be set.
> >>>
> >>> didn't get this point. you mean 'detach' will fail?
> >>
> >> Yes, because it tried to reset the flag in the process.
> >>
> >> This is something we can make user friendly - the detach succeeds, but
> >> the recurse flag is ignored and the recurse flag in the group is not
> >> reset unless it is the base group with the recurse flag (i.e., the
> >> parent is marked non-recurse).
> >
> > if we don't reset group flags to default it will be even more difficult
> > for users to use, since attach with recursive flag + immediate detach sets
> > some internal flag on the cgroup and user space has no way of
> > either querying this flag or deleting it.
>
> We have discussed this before -- the need to know which cgroup has a
> program and now what is the status of flags. That need is a different
> problem than this patch set.
>
> I'll address the reset of the flags below to keep that discussion together.
>
> >
> >>>
> >>>> 2. Program attached to 'b' is detached, recursive flag is set. Allowed.
> >>>
> >>> meaing that detach from 'b' has to pass recurse flag to be detached?
> >>> That's also odd.
> >>> imo detach should always succeed and the process doing detach
> >>> shouldn't need to know what flags were used in attach.
> >>
> >> Then, we agree to make it user friendly and handle resetting the recurse
> >> flag automatically.
> >
> > in that sense yes attach/delete pair should be side-effect free.
> >
> >>>> Process in 'b' opens a socket. No program attached to 'b' so no program
> >>>> is run. Recursive flag is set to program from 'a' is run. Stop.
> >>>>
> >>>> We should allow the recursive flag to be reset if the parent is not
> >>>> recursive allowing an unwind of settings applied. I'll add that change.
> >>>
> >>> I don't get this part.
> >>> Anyway looking forward to the next patch set with tests and comments like above.
> >>
> >> Per above discussion, you don't want 'a' run since it is not marked
> >> recurse. My last sentence is the user friendly part in resetting the
> >> flag in the cgroup.
> >
> > I'm still not grasping fully the semantics of what you're proposing.
> > You keep saying that override and recurse flags are indepedent, but
> > the more we talk the more it's clear that there is a complicated
> > relationship between them. Like no_override overrules everything, etc.
>
> yes, I have said that a few times. Override should block everything in
> terms of installing programs. If it is not enabled, the status of the
> recurse flag is not relevant at attach / detach time as the call should
> fail. So installing a program with the recurse flag only works if
> override is allowed.
>
>
> > I'm looking for the simplest to use logic. Not implementation.
> > Implementation can be complex, but uapi should be as simple to
> > explain and as simple to understand as possible.
> > So how about allowing recurse+overide combination only?
> > All descendents must be recurse+override too and
> > no program allowed to be set on parent unless it's recurse+override
> > as well. Then detach anywhere is simple, since all programs in
> > such chain are always recurse+override.
>
>
> Let's walk through examples based on the new ground rule - recursion
> stops at last cgroup with flag set.
>
> Assuming override is allowed ...
>
> ${MNT}/a/b/c/d
>
> - 'a' has no program
>
> - 'b' has a program, override allowed, recurse set
>
> - 'c' and 'd' inherit the program from 'b' by recursion, not inheritance
> (ie., bpf.effective is not updated with the program from 'b', but the
> recurse flag is set on 'c' and 'd').
>
> At this point 'c' and 'd' can ONLY take programs that are recursive.
>
> - 'c' gets a program installed
> - 'd' gets a program installed.
>
>
> Process in 'd' has programs run in this order: 'd', 'c', 'b'
>
>
> Now, program 'c' is detached. It is in the middle of the recursive set.
> It MUST keep the recurse flag set as it inherited the restriction from
> 'b'. The recurse flag on 'c' can ONLY be reset when the program is
> detached from 'b' as it is the start of the recursive chain.
>
> I'll stop here to make sure we agree on the above. Considering all
> permutations is a maze.
Agree on the above, but you're mixing semantics of the new recurse
flag and implementation of it. Ex: we don't have to copy this flag
from prog->attr into cgroup. So this reset or non-reset discussion
only makes sense in the context of your current implementation.
We can implement the logic differently. Like don't copy that flag
at all and at attach time walk parent->parent->parent and see
what programs are attached. All of them should have prog->attr & recurse_bit set
In such implementation detach from 'b' is a nop from reset/non-reset
point of view. When socket creation in 'c' is invoked the program
'c' is called first then the code keeps walking parents until root
invoking 'a' along the way.
I'm not saying it will be an efficient implementation. The point
is to discuss UAPI independent of implementation.
> ###
>
> Also, let's agree on this intention. Based on the new ground rule, I
> want to point out this example:
>
> If 'a' gets a program installed with no recurse flag set, ONLY processes
> in 'a' have the 'a' program run. Processes in groups 'b', 'c' and 'd'
> all stop at cgroup 'b' program.
I'm proposing that such situation should not be allowed to happen.
In a->b->c->d cgroup scenario if override+recurse prog attached to 'b'
then only the same override+recurse can be attached to c, d, a.
So at detach time there can be gaps (like only 'b' and 'd' have
override+recurse progs), but walking up until root from any point
will guarantee that only override+recurse programs are seen.
^ permalink raw reply
* net.ipv4.tcp_max_syn_backlog implementation
From: Harsha Chenji @ 2017-08-29 3:47 UTC (permalink / raw)
To: netdev
So I have ubuntu 12.04 x32 in a VM with syncookies turned off. I tried
to do a syn flood (with netwox) on 3 different processes. Each of them
returns a different value with netstat -na | grep -c RECV :
nc -l 5555 returns 16 (netcat-traditional)
apache2 port 80 returns 256
vsftpd on 21 returns 64.
net.ipv4.tcp_max_syn_backlog is 512.
Why do these different processes on different ports have different
queue lengths for incomplete connections? Where exactly in the kernel
is this decided?
^ permalink raw reply
* Re: [net-next] be2net: use shift instead of expensive divide
From: Joe Perches @ 2017-08-29 3:44 UTC (permalink / raw)
To: Zhang Shengju, sathya.perla, ajit.khaparde, sriharsha.basavapatna,
netdev
In-Reply-To: <1503976719-3063-1-git-send-email-zhangshengju@cmss.chinamobile.com>
On Tue, 2017-08-29 at 11:18 +0800, Zhang Shengju wrote:
> Replace shift instead of expensive divide.
This change is mostly pointless.
Any half-way decent compiler should produce the same object.
gcc emits the same object with the old code.
The AMAP_GET_BITS macro uses the "offsetof(struct, member) / 32"
style too.
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
[]
> @@ -2455,7 +2455,7 @@ static struct be_rx_compl_info *be_rx_compl_get(struct be_rx_obj *rxo)
>
> /* For checking the valid bit it is Ok to use either definition as the
> * valid bit is at the same position in both v0 and v1 Rx compl */
> - if (compl->dw[offsetof(struct amap_eth_rx_compl_v1, valid) / 32] == 0)
> + if (compl->dw[offsetof(struct amap_eth_rx_compl_v1, valid) >> 5] == 0)
etc...
^ permalink raw reply
* Re: Hooking on L4 Level with process information
From: Ravish Kumar @ 2017-08-29 3:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Networking
In-Reply-To: <20170828201900.625a10ed@xeon-e3>
Not convinced with this .
A process open a socket and that socket is associated with that
particular process unless it shares the file descriptors.
Can you explain why it is not related , at a time a socket will be
opened by a particular process.
On Tue, Aug 29, 2017 at 8:49 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 29 Aug 2017 07:34:51 +0530
> Ravish Kumar <ravishk2004@gmail.com> wrote:
>
>> Hi,
>>
>> I want to hook tcp/udp packets on L4 Layer and based on process
>> information , content want to deny or allow packets.
>>
>> Netfilter provides pre/post Routing hooks but not sure that will be
>> right place so thought of asking whether my approach is right.
>> Also how i can get process information whether this packet is send by
>> this process.
>>
>> Thoughts /source code reference would be appreciated.
>>
>> Regards,
>> Ravish
>
> There is not a 1:1 relationship between sockets/files and processes.
^ permalink raw reply
* RE: [patch net-next 2/3] net/sched: Change cls_flower to use IDR
From: Chris Mi @ 2017-08-29 3:25 UTC (permalink / raw)
To: Simon Horman
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
mawilcox@microsoft.com
In-Reply-To: <20170828113721.GA14697@vergenet.net>
> -----Original Message-----
> From: Simon Horman [mailto:simon.horman@netronome.com]
> Sent: Monday, August 28, 2017 7:37 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: netdev@vger.kernel.org; jhs@mojatatu.com;
> xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net;
> mawilcox@microsoft.com
> Subject: Re: [patch net-next 2/3] net/sched: Change cls_flower to use IDR
>
> On Mon, Aug 28, 2017 at 02:41:16AM -0400, Chris Mi wrote:
> > Currently, all filters with the same priority are linked in a doubly
> > linked list. Every filter should have a unique handle. To make the
> > handle unique, we need to iterate the list every time to see if the
> > handle exists or not when inserting a new filter. It is time-consuming.
> > For example, it takes about 5m3.169s to insert 64K rules.
> >
> > This patch changes cls_flower to use IDR. With this patch, it takes
> > about 0m1.127s to insert 64K rules. The improvement is huge.
>
> Very nice :)
>
> > But please note that in this testing, all filters share the same action.
> > If every filter has a unique action, that is another bottleneck.
> > Follow-up patch in this patchset addresses that.
> >
> > Signed-off-by: Chris Mi <chrism@mellanox.com>
> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> > net/sched/cls_flower.c | 55
> > +++++++++++++++++++++-----------------------------
> > 1 file changed, 23 insertions(+), 32 deletions(-)
> >
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index
> > bd9dab4..3d041d2 100644
> > --- a/net/sched/cls_flower.c
> > +++ b/net/sched/cls_flower.c
>
> ...
>
> > @@ -890,6 +870,7 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
> > struct cls_fl_filter *fnew;
> > struct nlattr **tb;
> > struct fl_flow_mask mask = {};
> > + unsigned long idr_index;
> > int err;
> >
> > if (!tca[TCA_OPTIONS])
> > @@ -920,13 +901,21 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
> > goto errout;
> >
> > if (!handle) {
> > - handle = fl_grab_new_handle(tp, head);
> > - if (!handle) {
> > - err = -EINVAL;
> > + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> > + 1, 0x80000000, GFP_KERNEL);
> > + if (err)
> > goto errout;
> > - }
> > + fnew->handle = idr_index;
> > + }
> > +
> > + /* user specifies a handle and it doesn't exist */
> > + if (handle && !fold) {
> > + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> > + handle, handle + 1, GFP_KERNEL);
> > + if (err)
> > + goto errout;
> > + fnew->handle = idr_index;
> > }
> > - fnew->handle = handle;
> >
> > if (tb[TCA_FLOWER_FLAGS]) {
> > fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
> > @@ -980,6 +969,8 @@ static int fl_change(struct net *net, struct sk_buff
> *in_skb,
> > *arg = fnew;
> >
> > if (fold) {
> > + fnew->handle = handle;
>
> Can it be the case that fold is non-NULL and handle is zero?
> The handling of that case seem to have changed in this patch.
I don't think that could happen. In function tc_ctl_tfilter(),
fl_get() will be called. If handle is zero, fl_get() will return NULL.
That means fold is NULL.
>
> > + idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
> > list_replace_rcu(&fold->list, &fnew->list);
> > tcf_unbind_filter(tp, &fold->res);
> > call_rcu(&fold->rcu, fl_destroy_filter);
> > --
> > 1.8.3.1
> >
^ 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