* Re: [PATCH v4 iproute2 2/7] rdma: Add dev object
From: Stephen Hemminger @ 2017-08-15 16:12 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-3-leonro@mellanox.com>
On Tue, 15 Aug 2017 16:00:15 +0300
Leon Romanovsky <leonro@mellanox.com> wrote:
> +
> +static const char *dev_caps_to_str(uint32_t idx)
> +{
> + uint64_t cap = 1 << idx;
> +
> + switch (cap) {
> + case RDMA_DEV_RESIZE_MAX_WR: return "RESIZE_MAX_WR";
> + case RDMA_DEV_BAD_PKEY_CNTR: return "BAD_PKEY_CNTR";
> + case RDMA_DEV_BAD_QKEY_CNTR: return "BAD_QKEY_CNTR";
> + case RDMA_DEV_RAW_MULTI: return "RAW_MULTI";
> + case RDMA_DEV_AUTO_PATH_MIG: return "AUTO_PATH_MIG";
> + case RDMA_DEV_CHANGE_PHY_PORT: return "CHANGE_PHY_POR";
> + case RDMA_DEV_UD_AV_PORT_ENFORCE: return "UD_AV_PORT_ENFORCE";
> + case RDMA_DEV_CURR_QP_STATE_MOD: return "CURR_QP_STATE_MOD";
> + case RDMA_DEV_SHUTDOWN_PORT: return "SHUTDOWN_PORT";
> + case RDMA_DEV_INIT_TYPE: return "INIT_TYPE";
> + case RDMA_DEV_PORT_ACTIVE_EVENT: return "PORT_ACTIVE_EVENT";
> + case RDMA_DEV_SYS_IMAGE_GUID: return "SYS_IMAGE_GUID";
> + case RDMA_DEV_RC_RNR_NAK_GEN: return "RC_RNR_NAK_GEN";
> + case RDMA_DEV_SRQ_RESIZE: return "SRQ_RESIZE";
> + case RDMA_DEV_N_NOTIFY_CQ: return "N_NOTIFY_CQ";
> + case RDMA_DEV_LOCAL_DMA_LKEY: return "LOCAL_DMA_LKEY";
> + case RDMA_DEV_MEM_WINDOW: return "MEM_WINDOW";
> + case RDMA_DEV_UD_IP_CSUM: return "UD_IP_CSUM";
> + case RDMA_DEV_UD_TSO: return "UD_TSO";
> + case RDMA_DEV_XRC: return "XRC";
> + case RDMA_DEV_MEM_MGT_EXTENSIONS: return "MEM_MGT_EXTENSIONS";
> + case RDMA_DEV_BLOCK_MULTICAST_LOOPBACK:
> + return "BLOCK_MULTICAST_LOOPBACK";
> + case RDMA_DEV_MEM_WINDOW_TYPE_2A: return "MEM_WINDOW_TYPE_2A";
> + case RDMA_DEV_MEM_WINDOW_TYPE_2B: return "MEM_WINDOW_TYPE_2B";
> + case RDMA_DEV_RC_IP_CSUM: return "RC_IP_CSUM";
> + case RDMA_DEV_RAW_IP_CSUM: return "RAW_IP_CSUM";
> + case RDMA_DEV_CROSS_CHANNEL: return "CROSS_CHANNEL";
> + case RDMA_DEV_MANAGED_FLOW_STEERING: return "MANAGED_FLOW_STEERING";
> + case RDMA_DEV_SIGNATURE_HANDOVER: return "SIGNATURE_HANDOVER";
> + case RDMA_DEV_ON_DEMAND_PAGING: return "ON_DEMAND_PAGING";
> + case RDMA_DEV_SG_GAPS_REG: return "SG_GAPS_REG";
> + case RDMA_DEV_VIRTUAL_FUNCTION: return "VIRTUAL_FUNCTION";
> + case RDMA_DEV_RAW_SCATTER_FCS: return "RAW_SCATTER_FCS";
> + case RDMA_DEV_RDMA_NETDEV_OPA_VNIC: return "RDMA_NETDEV_OPA_VNIC";
> + default: return "UNKNOWN";
> + }
Could this be a table in future versions?
^ permalink raw reply
* Re: [iproute PATCH 00/51] Fix potential issues detected by Coverity tool
From: Stephen Hemminger @ 2017-08-15 16:14 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170815160432.GF16375@orbyte.nwl.cc>
On Tue, 15 Aug 2017 18:04:32 +0200
Phil Sutter <phil@nwl.cc> wrote:
> On Tue, Aug 15, 2017 at 08:07:25AM -0700, Stephen Hemminger wrote:
> > On Sat, 12 Aug 2017 14:04:19 +0200
> > Phil Sutter <phil@nwl.cc> wrote:
> >
> > > Covscan really wasn't amused (indicated by the number of patches in this
> > > series). Try to make it happy.
> > >
> > > Phil Sutter (51):
> [...]
> > > 40 files changed, 230 insertions(+), 168 deletions(-)
> > >
> >
> > I am not amused by large patchsets either.
> > It takes more time to review, and one comment means the whole series
> > has to be redone.
>
> OK, so should I send each patch individually (unless related to others)?
>
> Thanks, Phil
Group like patches as needed. For example, all the ss patches.
^ permalink raw reply
* Fw: [Bug 196671] New: TUN(GET/ATTACH/DETACH)FILTER ioctls fail in a x86-32 process on an x86-64 kernel
From: Stephen Hemminger @ 2017-08-15 16:15 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Tue, 15 Aug 2017 14:21:36 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 196671] New: TUN(GET/ATTACH/DETACH)FILTER ioctls fail in a x86-32 process on an x86-64 kernel
https://bugzilla.kernel.org/show_bug.cgi?id=196671
Bug ID: 196671
Summary: TUN(GET/ATTACH/DETACH)FILTER ioctls fail in a x86-32
process on an x86-64 kernel
Product: Networking
Version: 2.5
Kernel Version: 4.12.0 (Linus)
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: robert@ocallahan.org
Regression: No
These ioctls take a 'struct sock_fprog' but there's no 32-bit compat code,
which has two consequences:
1) The ioctl numbers are incorrect for 32-bit processes running on a 64-bit
kernel. The userspace ioctl numbers use an 8-byte 'size' field but the kernel
expects a 16-byte size field, so the ioctls fail with EINVAL. You can hack
around this by handcoding the 64-bit ioctl number.
2) Userspace must supply a sock_fprog with the 64-bit layout.
Of course these workarounds assume you're running on a 64-bit kernel, so
they're no good if you might be running on a 32-bit kernel.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [PATCH 0/8] MIPS: BCM63XX: add and use clkdev lookup support
From: Florian Fainelli @ 2017-08-15 16:20 UTC (permalink / raw)
To: Jonas Gorski, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Ralf Baechle,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Kevin Cernekee,
Jiri Slaby, David S. Miller, Russell King
In-Reply-To: <20170802093429.12572-1-jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 08/02/2017 02:34 AM, Jonas Gorski wrote:
> This patchset adds and uses clckdev lookup support to name input clocks
> in various drivers more closely to their functions, or simplify their
> usage.
>
> Since most of these patches touch arch/mips, it probably makes most
> sense to go through the MIPS tree.
>
> The HSSPI driver was already updated previously to support a "pll"
> input with ff18e1ef04e2 ("spi/bcm63xx-hsspi: allow providing clock rate
> through a second clock"), so there is no need to touch it.
>
> This patch series is part of an effort to modernize BCM63XX and clean up
> its drivers to eventually make them usable with BMIPS and device tree.
For this entire series:
Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks!
>
> Jonas Gorski (8):
> MIPS: BCM63XX: add clkdev lookup support
> MIPS: BCM63XX: provide periph clock as refclk for uart
> tty/bcm63xx_uart: use refclk for the expected clock name
> tty/bcm63xx_uart: allow naming clock in device tree
> MIPS: BCM63XX: provide enet clocks as "enet" to the ethernet devices
> bcm63xx_enet: just use "enet" as the clock name
> MIPS: BCM63XX: move the HSSPI PLL HZ into its own clock
> MIPS: BMIPS: name the refclk clock for uart
>
> .../bindings/serial/brcm,bcm6345-uart.txt | 6 +
> arch/mips/Kconfig | 1 +
> arch/mips/bcm63xx/clk.c | 181 ++++++++++++++++-----
> arch/mips/boot/dts/brcm/bcm3368.dtsi | 2 +
> arch/mips/boot/dts/brcm/bcm63268.dtsi | 2 +
> arch/mips/boot/dts/brcm/bcm6328.dtsi | 2 +
> arch/mips/boot/dts/brcm/bcm6358.dtsi | 2 +
> arch/mips/boot/dts/brcm/bcm6362.dtsi | 2 +
> arch/mips/boot/dts/brcm/bcm6368.dtsi | 2 +
> drivers/net/ethernet/broadcom/bcm63xx_enet.c | 5 +-
> drivers/tty/serial/bcm63xx_uart.c | 6 +-
> 11 files changed, 168 insertions(+), 43 deletions(-)
>
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] New Chapter on CodingStyle .
From: Stephen Hemminger @ 2017-08-15 16:20 UTC (permalink / raw)
To: David Laight
Cc: 'Jonathan Corbet', Corcodel Marian,
netdev@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Realtek linux nic maintainers
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0055A0C@AcuExch.aculab.com>
On Tue, 15 Aug 2017 10:42:39 +0000
David Laight <David.Laight@ACULAB.COM> wrote:
> From: Jonathan Corbet
> > Sent: 12 August 2017 15:55
> ...
> > > + Chapter 20: Put values on initialisers without exception
> > > +
> > > +When declaring variables on functions must put values:
> >
> > Thanks for sending a patch for the kernel's documentation.
> > Unfortunately, I can't accept this patch for a couple of reasons:
> ...
> > - The coding style document is there to describe the community's
> > standards for kernel code. It is *not* a mechanism for imposing new
> > standards. If you really think that the kernel community should adopt
> > this rule, you will need to argue for it on the mailing lists. I will
> > say, though, that I do not expect that this effort would be successful.
>
> I'd even go as far as suggesting almost the opposite.
> Declarations should only have initialisers if the value is constant.
Yup. This new rule sound like something taught to people in coding schools.
But initializing everything defeats the compiler detection of uninitialized variables
which is more useful for catching errors.
^ permalink raw reply
* RE: [PATCH v4 iproute2 2/7] rdma: Add dev object
From: David Laight @ 2017-08-15 16:23 UTC (permalink / raw)
To: 'Stephen Hemminger', Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Dennis Dalessandro, Jason Gunthorpe, Jiri Pirko, Ariel Almog,
Linux Netdev
In-Reply-To: <20170815091205.2c39ea91@xeon-e3>
From: Stephen Hemminger
> Sent: 15 August 2017 17:12
> On Tue, 15 Aug 2017 16:00:15 +0300
> Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>
> > +
> > +static const char *dev_caps_to_str(uint32_t idx)
> > +{
> > + uint64_t cap = 1 << idx;
> > +
> > + switch (cap) {
> > + case RDMA_DEV_RESIZE_MAX_WR: return "RESIZE_MAX_WR";
> > + case RDMA_DEV_BAD_PKEY_CNTR: return "BAD_PKEY_CNTR";
...
> > + case RDMA_DEV_RAW_SCATTER_FCS: return "RAW_SCATTER_FCS";
> > + case RDMA_DEV_RDMA_NETDEV_OPA_VNIC: return "RDMA_NETDEV_OPA_VNIC";
> > + default: return "UNKNOWN";
> > + }
>
> Could this be a table in future versions?
Potentially you could define the constants using some pre-processor
'magic' that would create the table for you.
Something like (but not compiled):
#define RDMA_DEV_FLAGS(x) \
x(RESIZE_MAX_WR, 0) \
x(BAD_PKEY_CNTR, 1) \
(continue for all the bits)
#define RDMA_DEV_ENUM(name, bit_no) RDMA_DEV_##name = BIT(bit_no),
enum {RDMA_DEV_FLAGS(RDMA_DEV_ENUM)};
#undef RDMA_DEV_ENUM
#define RDMA_DEV_NAMES(name, bit_no) [bit_no] = #name,
static const char rdma_dev_names[] = {RDMA_DEV_FLAGS(RDMA_DEV_NAMES)};
David
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH net-next 06/11] net: dsa: debugfs: add port registers
From: Woojung.Huh @ 2017-08-15 16:28 UTC (permalink / raw)
To: vivien.didelot, netdev
Cc: linux-kernel, kernel, davem, f.fainelli, andrew, privat, john,
sean.wang, volodymyr.bendiuga, nikita.yoush, maxime, cphealy,
muvarov, eichest, jcobham, jbe, tobias
In-Reply-To: <20170814222242.10643-7-vivien.didelot@savoirfairelinux.com>
Vivien,
> Subject: [PATCH net-next 06/11] net: dsa: debugfs: add port registers
>
> Add a debug filesystem "regs" entry to query a port's hardware registers
> through the .get_regs_len and .get_regs_len switch operations.
>
> This is very convenient because it allows one to dump the registers of
> DSA links, which are not exposed to userspace.
This series will be very useful to get various debug information.
Do you have any plan to expand 32bit register access and switch-level registers
In addition to port-level registers?
> +static void dsa_debugfs_regs_read_count(struct dsa_switch *ds, int id,
> + struct seq_file *seq, int count)
> +{
> + u16 data[count * ETH_GSTRING_LEN];
I think this should be u16 data[count] because it is value of registers.
> + struct ethtool_regs regs;
> + int i;
> +
> + ds->ops->get_regs(ds, id, ®s, data);
> +
> + for (i = 0; i < count / 2; i++)
> + seq_printf(seq, "%2d: %04x\n", i, data[i]);
> +}
> +
> +static int dsa_debugfs_regs_read(struct dsa_switch *ds, int id,
> + struct seq_file *seq)
> +{
> + int count;
> +
> + if (!ds->ops->get_regs_len || !ds->ops->get_regs)
> + return -EOPNOTSUPP;
> +
> + count = ds->ops->get_regs_len(ds, id);
> + if (count < 0)
> + return count;
Because get_regs_len returns length than count per mv88e6xxx/chip.c,
it requires some math before passing to dsa_debugfs_regs_read_count().
It does "count/2" in dsa-debugfs_regs_read_count(), however,
As commented above, "count" is used at "u16 data[...]".
So, it would be nice to match with name. Ie, count or size/length.
Thanks.
Woojung
^ permalink raw reply
* Re: [iproute PATCH 05/51] iplink_can: Prevent overstepping array bounds
From: Phil Sutter @ 2017-08-15 16:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170815081049.10ef8e06@xeon-e3>
On Tue, Aug 15, 2017 at 08:10:49AM -0700, Stephen Hemminger wrote:
> On Sat, 12 Aug 2017 14:04:24 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > can_state_names array contains at most CAN_STATE_MAX fields, so allowing
> > an index to it to be equal to that number is wrong. While here, also
> > make sure the array is indeed that big so nothing bad happens if
> > CAN_STATE_MAX ever increases.
>
> No more speculative bug fixes.
I don't think a bit of speculation regarding forwards-compatibility is a
bad thing per se. In this case it is about the possibility for kernel
code to add a new state to enum can_state.
Older ip binaries will allow an index of CAN_STATE_MAX and therefore
access data beyond end of can-state_names array.
If you update linux headers but forget to add the new state to
can_state_names array, the same will happen even if the sanity check for
'state' value is being fixed as by my patch.
By specifying the size of can_state_names array upon definition,
can_print_opt() will just print a null pointer which printf() can
handle.
Cheers, Phil
^ permalink raw reply
* 100% CPU load when generating traffic to destination network that nexthop is not reachable
From: Paweł Staszewski @ 2017-08-15 16:30 UTC (permalink / raw)
To: Linux Kernel Network Developers
Hi
Doing some tests i discovered that when traffic is send by pktgen to
forwarding host where nexthop for destination network on forwarding
router is not reachable i have 100% cpu on all cores and perf top show
mostly:
77.19% [kernel] [k] queued_spin_lock_slowpath
10.20% [kernel] [k] acpi_processor_ffh_cstate_enter
1.41% [kernel] [k] queued_write_lock_slowpath
Configuration of forwarding host below:
ip a
Receiving interface:
8: enp175s0f0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state
UP group default qlen 1000
link/ether 0c:c4:7a:d8:5d:1c brd ff:ff:ff:ff:ff:ff
inet 10.0.0.1/30 scope global enp175s0f0
valid_lft forever preferred_lft forever
inet6 fe80::ec4:7aff:fed8:5d1c/64 scope link
valid_lft forever preferred_lft forever
Transmitting vlans (binded to: enp175s0f1)
12: vlan1000@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
qdisc noqueue state UP group default qlen 1000
link/ether 0c:c4:7a:d8:5d:1d brd ff:ff:ff:ff:ff:ff
inet 10.10.0.1/30 scope global vlan1000
valid_lft forever preferred_lft forever
inet6 fe80::ec4:7aff:fed8:5d1d/64 scope link
valid_lft forever preferred_lft forever
13: vlan1001@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
qdisc noqueue state UP group default qlen 1000
link/ether 0c:c4:7a:d8:5d:1d brd ff:ff:ff:ff:ff:ff
inet 10.10.1.1/30 scope global vlan1001
valid_lft forever preferred_lft forever
inet6 fe80::ec4:7aff:fed8:5d1d/64 scope link
valid_lft forever preferred_lft forever
14: vlan1002@enp175s0f1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
qdisc noqueue state UP group default qlen 1000
link/ether 0c:c4:7a:d8:5d:1d brd ff:ff:ff:ff:ff:ff
inet 10.10.2.1/30 scope global vlan1002
valid_lft forever preferred_lft forever
inet6 fe80::ec4:7aff:fed8:5d1d/64 scope link
valid_lft forever preferred_lft forever
Routing table:
10.0.0.0/30 dev enp175s0f0 proto kernel scope link src 10.0.0.1
10.10.0.0/30 dev vlan1000 proto kernel scope link src 10.10.0.1
10.10.1.0/30 dev vlan1001 proto kernel scope link src 10.10.1.1
10.10.2.0/30 dev vlan1002 proto kernel scope link src 10.10.2.1
172.16.0.0/24 via 10.10.0.2 dev vlan1000
172.16.1.0/24 via 10.10.1.2 dev vlan1001
172.16.2.0/24 via 10.10.2.2 dev vlan1002
pktgen is transmitting packets to this forwarding hosts and generating
random destinations from ip range:
pg_set $dev "dst_min 172.16.0.1"
pg_set $dev "dst_max 172.16.2.255"
So when packets with destination network 172.16.0.0/24 are reaching
forwarding host then are routed via 10.10.0.2 dev vlan1000
for packets with destination network 172.16.1.0/24 forwarding host
routing them via 10.10.1.2 dev vlan1001
and last network 172.16.2.0/24 is routed via 10.10.2.2 dev vlan1002
Normally when situation is like this:
ip neigh ls dev vlan1000
10.10.0.2 lladdr ac:1f:6b:2c:18:89 REACHABLE
ip neigh ls dev vlan1001
10.10.1.2 lladdr ac:1f:6b:2c:18:89 REACHABLE
ip neigh ls dev vlan1002
10.10.2.2 lladdr ac:1f:6b:2c:18:89 REACHABLE
There is no problem router is receiving 11Mpps and forwarding then
equally to vlans:
bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
input: /proc/net/dev type: rate
- iface Rx Tx Total
==============================================================================
vlan1002: 0.00 P/s 3877006.00 P/s 3877006.00 P/s
vlan1001: 0.00 P/s 3877234.75 P/s 3877234.75 P/s
enp175s0f0: 11962601.00 P/s 0.00 P/s 11962601.00 P/s
vlan1000: 0.00 P/s 3862602.00 P/s 3862602.00 P/s
------------------------------------------------------------------------------
total: 11962601.00 P/s 11616843.00 P/s 23579444.00 P/s
And perf top shows like this:
PerfTop: 210522 irqs/sec kernel:99.7% exact: 0.0% [4000Hz
cycles], (all, 56 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
26.98% [kernel] [k] do_raw_spin_lock
7.69% [kernel] [k] acpi_processor_ffh_cstate_enter
4.92% [kernel] [k] fib_table_lookup
4.28% [mlx5_core] [k] mlx5e_xmit
4.01% [mlx5_core] [k] mlx5e_handle_rx_cqe
2.71% [kernel] [k] virt_to_head_page
2.21% [kernel] [k] tasklet_action
1.87% [mlx5_core] [k] mlx5_eq_int
1.58% [kernel] [k] ipt_do_table
1.55% [mlx5_core] [k] mlx5e_poll_tx_cq
1.53% [kernel] [k] irq_entries_start
1.48% [kernel] [k] __dev_queue_xmit
1.44% [kernel] [k] __build_skb
1.30% [mlx5_core] [k] eq_update_ci
1.20% [kernel] [k] read_tsc
1.10% [kernel] [k] ip_finish_output2
1.06% [kernel] [k] ip_rcv
1.02% [kernel] [k] netif_skb_features
1.01% [mlx5_core] [k] mlx5_cqwq_get_cqe
0.95% [kernel] [k] __netif_receive_skb_core
But when i will disable any vlan on the switch - for example I will do
this for vlan1002
(Forwarding host is connected thru switch where are vlans to the sink host)
root@cumulus:~# ip link set down dev vlan1002.49
root@cumulus:~# ip link set down dev vlan1002.3
root@cumulus:~# ip link set down dev brtest1002
Wait for fdb to expire on switch.
there is incomplete arp on interface vlan1002
ip neigh ls dev vlan1002
10.10.2.2 INCOMPLETE
pktgen is still pushing traffic with packets that destination network is
172.16.2.0/24
and we have 100% cpu with pps below:
bwm-ng v0.6.1 (probing every 0.500s), press 'h' for help
input: /proc/net/dev type: rate
| iface Rx Tx Total
==============================================================================
vlan1002: 0.00 P/s 1.99 P/s
1.99 P/s
vlan1001: 0.00 P/s 717227.12 P/s 717227.12 P/s
enp175s0f0: 2713679.25 P/s 0.00 P/s 2713679.25 P/s
vlan1000: 0.00 P/s 716145.44 P/s 716145.44 P/s
------------------------------------------------------------------------------
total: 2713679.25 P/s 1433374.50 P/s 4147054.00 P/s
with perf top:
PerfTop: 218506 irqs/sec kernel:99.7% exact: 0.0% [4000Hz
cycles], (all, 56 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
91.45% [kernel] [k] queued_spin_lock_slowpath
1.71% [kernel] [k] queued_write_lock_slowpath
0.46% [kernel] [k] ip_finish_output2
0.44% [mlx5_core] [k] mlx5e_handle_rx_cqe
0.43% [kernel] [k] fib_table_lookup
0.40% [kernel] [k] do_raw_spin_lock
0.35% [kernel] [k] __neigh_event_send
0.33% [kernel] [k] dst_release
0.26% [kernel] [k] queued_write_lock
0.22% [mlx5_core] [k] mlx5_cqwq_get_cqe
0.22% [mlx5_core] [k] mlx5e_xmit
0.19% [kernel] [k] virt_to_head_page
0.18% [kernel] [k] page_frag_free
[...]
^ permalink raw reply
* Re: [PATCH v2] sctp: fully initialize the IPv6 address in sctp_v6_to_addr()
From: Marcelo Ricardo Leitner @ 2017-08-15 16:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, hideaki.yoshifuji, glider, dvyukov, kcc, edumazet,
lucien.xin, vyasevich, linux-kernel, linux-sctp, netdev, yoshfuji
In-Reply-To: <1502811469.4936.84.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Aug 15, 2017 at 08:37:49AM -0700, Eric Dumazet wrote:
> On Tue, 2017-08-15 at 12:05 -0300, Marcelo Ricardo Leitner wrote:
>
> > Ok, but I should see a difference in the generated code, right?
>
> Depends on the compiler. Have you tried older versions ?
>
This was with gcc 6.4.1, fc25 standard. Only tested with it and didn't
check clang either.
> One argument is that following struct member definition eases code
> review.
>
> (It is easier to catch a field init is missing)
And a good one, yes.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH v2 net-next] bpf/verifier: track liveness for pruning
From: Daniel Borkmann via iovisor-dev @ 2017-08-15 16:33 UTC (permalink / raw)
To: Edward Cree, davem-fT/PcQaiUtIeIZ0/mPfg9Q, Alexei Starovoitov,
Alexei Starovoitov
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3c245ef4-3161-0e5a-3708-6b9b47db01cd-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
On 08/15/2017 03:53 PM, Edward Cree wrote:
> State of a register doesn't matter if it wasn't read in reaching an exit;
> a write screens off all reads downstream of it from all explored_states
> upstream of it.
> This allows us to prune many more branches; here are some processed insn
> counts for some Cilium programs:
> Program before after
> bpf_lb_opt_-DLB_L3.o 6515 3361
> bpf_lb_opt_-DLB_L4.o 8976 5176
> bpf_lb_opt_-DUNKNOWN.o 2960 1137
> bpf_lxc_opt_-DDROP_ALL.o 95412 48537
> bpf_lxc_opt_-DUNKNOWN.o 141706 78718
> bpf_netdev.o 24251 17995
> bpf_overlay.o 10999 9385
>
> The runtime is also improved; here are 'time' results in ms:
> Program before after
> bpf_lb_opt_-DLB_L3.o 24 6
> bpf_lb_opt_-DLB_L4.o 26 11
> bpf_lb_opt_-DUNKNOWN.o 11 2
> bpf_lxc_opt_-DDROP_ALL.o 1288 139
> bpf_lxc_opt_-DUNKNOWN.o 1768 234
> bpf_netdev.o 62 31
> bpf_overlay.o 15 13
>
> Signed-off-by: Edward Cree <ecree-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
[...]
> @@ -1639,10 +1675,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
> }
>
> /* reset caller saved regs */
> - for (i = 0; i < CALLER_SAVED_REGS; i++)
> + for (i = 0; i < CALLER_SAVED_REGS; i++) {
> mark_reg_not_init(regs, caller_saved[i]);
> + check_reg_arg(env, i, DST_OP_NO_MARK);
Ah, I oversaw that earlier, this needs to be: s/i/caller_saved[i]/
> + }
>
> /* update return register */
> + check_reg_arg(env, BPF_REG_0, DST_OP_NO_MARK);
We could leave this for clarity, but ...
> if (fn->ret_type == RET_INTEGER) {
> /* sets type to SCALAR_VALUE */
> mark_reg_unknown(regs, BPF_REG_0);
[...]
>
> /* reset caller saved regs to unreadable */
> - for (i = 0; i < CALLER_SAVED_REGS; i++)
> + for (i = 0; i < CALLER_SAVED_REGS; i++) {
> mark_reg_not_init(regs, caller_saved[i]);
> + check_reg_arg(env, i, DST_OP_NO_MARK);
caller_saved[i]
> + }
>
> /* mark destination R0 register as readable, since it contains
> - * the value fetched from the packet
> + * the value fetched from the packet.
> + * Already marked as written above.
... then it should be here as well. Other option is to leave out
both BPF_REG_0 since covered by caller_saved[] already.
> */
> mark_reg_unknown(regs, BPF_REG_0);
> return 0;
> @@ -3194,7 +3236,11 @@ static bool regsafe(struct bpf_reg_state *rold,
> struct bpf_reg_state *rcur,
> bool varlen_map_access, struct idpair *idmap)
> {
> - if (memcmp(rold, rcur, sizeof(*rold)) == 0)
> + if (!(rold->live & REG_LIVE_READ))
> + /* explored state didn't use this */
> + return true;
^ permalink raw reply
* Re: [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile
From: Randy Dunlap @ 2017-08-15 16:41 UTC (permalink / raw)
To: Neal Cardwell, mohamedalrshah
Cc: David Miller, Netdev, Linus Torvalds, LKML, Mohamed Othman,
Borhanuddin Ali, Zurina Hanapi
In-Reply-To: <CADVnQymS_n=ihTaBCx-A6rw2Xxj264ua5xK7mKCppe5r_aR+ig@mail.gmail.com>
On 08/15/2017 06:51 AM, Neal Cardwell wrote:
> On Tue, Aug 15, 2017 at 9:08 AM, mohamedalrshah
> <mohamed.a.alrshah@ieee.org> wrote:
>
>> +static void agilesdtcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
>> +{
>> + struct tcp_sock *tp = tcp_sk(sk);
>> + struct agilesdtcp *ca = inet_csk_ca(sk);
>> + u32 inc_factor;
>> + u32 ca_inc;
>> + u32 current_gap, total_gap;
>
> For coding style, please order local variable declarations from
> longest to shortest line, also know as Reverse Christmas Tree Format.
Per what coding style, please? This is not in current coding style nor in the
netdev-FAQ exceptions.
--
~Randy
^ permalink raw reply
* Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy()
From: Phil Sutter @ 2017-08-15 16:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170815081555.104dab6e@xeon-e3>
On Tue, Aug 15, 2017 at 08:15:55AM -0700, Stephen Hemminger wrote:
> On Sat, 12 Aug 2017 14:04:40 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > Both addattr_l() and rta_addattr_l() may be called with NULL data
> > pointer and 0 alen parameters. Avoid calling memcpy() in that case.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
>
> What are you fixing. memcpy(dest, NULL, 0) should be harmless NOP
Yes, if that turns into a NOP this patch is not needed.
Thanks, Phil
^ permalink raw reply
* Re: [PATCH v4 iproute2 2/7] rdma: Add dev object
From: Leon Romanovsky @ 2017-08-15 16:43 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Doug Ledford, linux-rdma, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815091205.2c39ea91@xeon-e3>
[-- Attachment #1: Type: text/plain, Size: 3156 bytes --]
On Tue, Aug 15, 2017 at 09:12:05AM -0700, Stephen Hemminger wrote:
> On Tue, 15 Aug 2017 16:00:15 +0300
> Leon Romanovsky <leonro@mellanox.com> wrote:
>
> > +
> > +static const char *dev_caps_to_str(uint32_t idx)
> > +{
> > + uint64_t cap = 1 << idx;
> > +
> > + switch (cap) {
> > + case RDMA_DEV_RESIZE_MAX_WR: return "RESIZE_MAX_WR";
> > + case RDMA_DEV_BAD_PKEY_CNTR: return "BAD_PKEY_CNTR";
> > + case RDMA_DEV_BAD_QKEY_CNTR: return "BAD_QKEY_CNTR";
> > + case RDMA_DEV_RAW_MULTI: return "RAW_MULTI";
> > + case RDMA_DEV_AUTO_PATH_MIG: return "AUTO_PATH_MIG";
> > + case RDMA_DEV_CHANGE_PHY_PORT: return "CHANGE_PHY_POR";
> > + case RDMA_DEV_UD_AV_PORT_ENFORCE: return "UD_AV_PORT_ENFORCE";
> > + case RDMA_DEV_CURR_QP_STATE_MOD: return "CURR_QP_STATE_MOD";
> > + case RDMA_DEV_SHUTDOWN_PORT: return "SHUTDOWN_PORT";
> > + case RDMA_DEV_INIT_TYPE: return "INIT_TYPE";
> > + case RDMA_DEV_PORT_ACTIVE_EVENT: return "PORT_ACTIVE_EVENT";
> > + case RDMA_DEV_SYS_IMAGE_GUID: return "SYS_IMAGE_GUID";
> > + case RDMA_DEV_RC_RNR_NAK_GEN: return "RC_RNR_NAK_GEN";
> > + case RDMA_DEV_SRQ_RESIZE: return "SRQ_RESIZE";
> > + case RDMA_DEV_N_NOTIFY_CQ: return "N_NOTIFY_CQ";
> > + case RDMA_DEV_LOCAL_DMA_LKEY: return "LOCAL_DMA_LKEY";
> > + case RDMA_DEV_MEM_WINDOW: return "MEM_WINDOW";
> > + case RDMA_DEV_UD_IP_CSUM: return "UD_IP_CSUM";
> > + case RDMA_DEV_UD_TSO: return "UD_TSO";
> > + case RDMA_DEV_XRC: return "XRC";
> > + case RDMA_DEV_MEM_MGT_EXTENSIONS: return "MEM_MGT_EXTENSIONS";
> > + case RDMA_DEV_BLOCK_MULTICAST_LOOPBACK:
> > + return "BLOCK_MULTICAST_LOOPBACK";
> > + case RDMA_DEV_MEM_WINDOW_TYPE_2A: return "MEM_WINDOW_TYPE_2A";
> > + case RDMA_DEV_MEM_WINDOW_TYPE_2B: return "MEM_WINDOW_TYPE_2B";
> > + case RDMA_DEV_RC_IP_CSUM: return "RC_IP_CSUM";
> > + case RDMA_DEV_RAW_IP_CSUM: return "RAW_IP_CSUM";
> > + case RDMA_DEV_CROSS_CHANNEL: return "CROSS_CHANNEL";
> > + case RDMA_DEV_MANAGED_FLOW_STEERING: return "MANAGED_FLOW_STEERING";
> > + case RDMA_DEV_SIGNATURE_HANDOVER: return "SIGNATURE_HANDOVER";
> > + case RDMA_DEV_ON_DEMAND_PAGING: return "ON_DEMAND_PAGING";
> > + case RDMA_DEV_SG_GAPS_REG: return "SG_GAPS_REG";
> > + case RDMA_DEV_VIRTUAL_FUNCTION: return "VIRTUAL_FUNCTION";
> > + case RDMA_DEV_RAW_SCATTER_FCS: return "RAW_SCATTER_FCS";
> > + case RDMA_DEV_RDMA_NETDEV_OPA_VNIC: return "RDMA_NETDEV_OPA_VNIC";
> > + default: return "UNKNOWN";
> > + }
>
> Could this be a table in future versions?
The device capability is 64 bit field -> the table will have 64 entry,
while most of them should be marked as "UNKNOWN". Because we have holes
[1] in the fields (for example bit 9), I can't use simple
"if (idx > max_value) return "uknown"" logic.
So the answer is yes, I can do it as a table, but it won't look better than
it is now :)
By writing "future versions", are you asking me to resubmit? or followup patch
will be possible solution too?
Thanks
[1] http://marc.info/?l=linux-rdma&m=150278850520856&w=2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-15 16:45 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Matthew Dawson, Network Development, Macieira, Thiago
In-Reply-To: <1502811641.3475.7.camel@redhat.com>
On Tue, Aug 15, 2017 at 11:40 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Mon, 2017-08-14 at 21:35 -0400, Willem de Bruijn wrote:
>> It is not infeasible to fix that and fix up all callers, as Matthew's
>> patch does. But perhaps this patch is simpler to reason about. Thoughts?
>>
>> +static inline bool sk_peek_at_offset(struct sock *sk)
>> +{
>> + return READ_ONCE(sk->sk_peek_off) >= 0;
>> +}
>> +
>> static inline int sk_peek_offset(struct sock *sk, int flags)
>> {
>> if (unlikely(flags & MSG_PEEK)) {
>> diff --git a/net/core/datagram.c b/net/core/datagram.c
>> index ee5647bd91b3..30b53932af73 100644
>> --- a/net/core/datagram.c
>> +++ b/net/core/datagram.c
>> @@ -175,12 +175,14 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
>> *last = queue->prev;
>> skb_queue_walk(queue, skb) {
>> if (flags & MSG_PEEK) {
>> - if (_off >= skb->len && (skb->len || _off ||
>> - skb->peeked)) {
>> + if (_off >= skb->len && sk_peek_at_offset(sk) &&
>> + (skb->len || _off || skb->peeked)) {
>
> I like the idea a lot, but I think/fear that bad things could happen
> since sk_peek_off is read twice at different times: one in
> sk_peek_offset() and one in the above chunk.
>
> If the above is not an issue (I am not sure) I'm very fine with this
> approach.
Good point. I don't think we need to read it in the callers at all.
Then int *off is only used to return to offset within the skb back
to the callers.
Though it should be read only once here, outside the loop.
Something like (also untested):
diff --git a/net/core/datagram.c b/net/core/datagram.c
index ee5647bd91b3..06bad8726612 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -170,13 +170,15 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
struct sk_buff **last)
{
struct sk_buff *skb;
- int _off = *off;
+ int _off;
+ bool peek_at_off;
+ _off = sk_peek_offset(sk, flags);
+ peek_at_off = _off >= 0;
*last = queue->prev;
skb_queue_walk(queue, skb) {
if (flags & MSG_PEEK) {
- if (_off >= skb->len && (skb->len || _off ||
- skb->peeked)) {
+ if (peek_at_off && off >= skb->len &&
+ (skb->len || _off || skb->peeked)) {
_off -= skb->len;
continue;
}
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index a7c804f73990..4b51b9853406 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1574,7 +1574,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr
*msg, size_t len, int noblock,
return ip_recv_error(sk, msg, len, addr_len);
try_again:
- peeking = off = sk_peek_offset(sk, flags);
+ peeking = flags & MSG_PEEK;
skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
if (!skb)
return err;
The explicit peek_at_off is a bit verbose, but we need to be careful
about signedness between _off and skb->len.
> For the record, I thought something like the following (uncomplete,
> does not even compile):
> ---
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 8b13db5163cc..5085cf003b88 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -286,6 +286,7 @@ struct ucred {
> #define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
> #define MSG_BATCH 0x40000 /* sendmmsg(): more messages coming */
> #define MSG_EOF MSG_FIN
> +#define MSG_PEEK_OFF 0x80000
Yes, that also works well.
I'm afraid about exhausting the MSG_* flag space here for a
feature that is not exposed to userspace. We don't have many flags
left. We could shadow an existing flag that is unused in this context.
There is another difference between reading sk_peek_offset in the
caller or in __skb_try_recv_from_queue. The latter is called repeatedly
when it returns NULL. Each call can modify *off. I believe that it needs
to restart with _off at sk->sk_peek_off each time, as it restarts from the
head of the queue each time.
>
> #define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
> #define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 7c0632c7e870..e75e024b55d2 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -504,12 +504,14 @@ enum sk_pacing {
>
> int sk_set_peek_off(struct sock *sk, int val);
>
> -static inline int sk_peek_offset(struct sock *sk, int flags)
> +static inline int sk_peek_offset(struct sock *sk, int *flags)
> {
> - if (unlikely(flags & MSG_PEEK)) {
> + if (unlikely(*flags & MSG_PEEK)) {
> s32 off = READ_ONCE(sk->sk_peek_off);
> - if (off >= 0)
> + if (off >= 0) {
> + *flags |= MSG_PEEK_OFF;
> return off;
> + }
> }
>
> return 0;
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index ee5647bd91b3..91e1d014d64c 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -175,8 +175,8 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> *last = queue->prev;
> skb_queue_walk(queue, skb) {
> if (flags & MSG_PEEK) {
> - if (_off >= skb->len && (skb->len || _off ||
> - skb->peeked)) {
> + if (flags & MSG_PEEK_OFF && _off >= skb->len &&
> + (skb->len || _off || skb->peeked)) {
> _off -= skb->len;
> continue;
> }
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index a7c804f73990..7e1bcd3500f4 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1574,7 +1574,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
> return ip_recv_error(sk, msg, len, addr_len);
>
> try_again:
> - peeking = off = sk_peek_offset(sk, flags);
> + peeking = off = sk_peek_offset(sk, &flags);
> skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
> if (!skb)
> return err;
> ---
> Paolo
^ permalink raw reply related
* Re: [PATCH v4 iproute2 2/7] rdma: Add dev object
From: Leon Romanovsky @ 2017-08-15 16:47 UTC (permalink / raw)
To: David Laight
Cc: 'Stephen Hemminger', Doug Ledford,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Dennis Dalessandro, Jason Gunthorpe, Jiri Pirko, Ariel Almog,
Linux Netdev
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005604E-VkEWCZq2GCInGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
On Tue, Aug 15, 2017 at 04:23:11PM +0000, David Laight wrote:
> From: Stephen Hemminger
> > Sent: 15 August 2017 17:12
> > On Tue, 15 Aug 2017 16:00:15 +0300
> > Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> >
> > > +
> > > +static const char *dev_caps_to_str(uint32_t idx)
> > > +{
> > > + uint64_t cap = 1 << idx;
> > > +
> > > + switch (cap) {
> > > + case RDMA_DEV_RESIZE_MAX_WR: return "RESIZE_MAX_WR";
> > > + case RDMA_DEV_BAD_PKEY_CNTR: return "BAD_PKEY_CNTR";
> ...
> > > + case RDMA_DEV_RAW_SCATTER_FCS: return "RAW_SCATTER_FCS";
> > > + case RDMA_DEV_RDMA_NETDEV_OPA_VNIC: return "RDMA_NETDEV_OPA_VNIC";
> > > + default: return "UNKNOWN";
> > > + }
> >
> > Could this be a table in future versions?
>
> Potentially you could define the constants using some pre-processor
> 'magic' that would create the table for you.
> Something like (but not compiled):
>
> #define RDMA_DEV_FLAGS(x) \
> x(RESIZE_MAX_WR, 0) \
> x(BAD_PKEY_CNTR, 1) \
> (continue for all the bits)
>
> #define RDMA_DEV_ENUM(name, bit_no) RDMA_DEV_##name = BIT(bit_no),
> enum {RDMA_DEV_FLAGS(RDMA_DEV_ENUM)};
> #undef RDMA_DEV_ENUM
>
> #define RDMA_DEV_NAMES(name, bit_no) [bit_no] = #name,
> static const char rdma_dev_names[] = {RDMA_DEV_FLAGS(RDMA_DEV_NAMES)};
>
David,
How should I handle "uknown" fields without names?
Thanks
> David
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [iproute PATCH 50/51] Check user supplied interface name lengths
From: Phil Sutter @ 2017-08-15 16:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170815090945.62b2c4d7@xeon-e3>
On Tue, Aug 15, 2017 at 09:09:45AM -0700, Stephen Hemminger wrote:
> On Sat, 12 Aug 2017 14:05:09 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > +void assert_valid_dev_name(const char *, const char *);
>
> Not a fan of long function names.
> “I have only made this letter longer because I have not had the time to make it shorter."
:)
> Maybe just add a new function addattr_ifname() and add the checking there?
It is not only about netlink attributes - these are in fact
unproblematic, since they allow for interface names longer than
IFNAMSIZ-1 and the kernel will then reject. The length check has to
happen before copying into any IFNAMSIZ-sized buffer takes place (e.g.
ifr_name of struct ifreq). Logically I would prefer to perform the
checks right at the point user input parsing takes place since it
belongs there.
I could rename the function to 'ifname_valid' instead? I liked having
'assert' in the name though, since the function calls exit() on error.
Thanks, Phil
^ permalink raw reply
* RE: [PATCH] New Chapter on CodingStyle .
From: David Laight @ 2017-08-15 16:55 UTC (permalink / raw)
To: 'Stephen Hemminger'
Cc: 'Jonathan Corbet', Corcodel Marian,
netdev@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Realtek linux nic maintainers
In-Reply-To: <20170815092030.7ec82ace@xeon-e3>
From: Stephen Hemminger
> Sent: 15 August 2017 17:21
> On Tue, 15 Aug 2017 10:42:39 +0000
> David Laight <David.Laight@ACULAB.COM> wrote:
>
> > From: Jonathan Corbet
> > > Sent: 12 August 2017 15:55
> > ...
> > > > + Chapter 20: Put values on initialisers without exception
> > > > +
> > > > +When declaring variables on functions must put values:
> > >
> > > Thanks for sending a patch for the kernel's documentation.
> > > Unfortunately, I can't accept this patch for a couple of reasons:
> > ...
> > > - The coding style document is there to describe the community's
> > > standards for kernel code. It is *not* a mechanism for imposing new
> > > standards. If you really think that the kernel community should adopt
> > > this rule, you will need to argue for it on the mailing lists. I will
> > > say, though, that I do not expect that this effort would be successful.
> >
> > I'd even go as far as suggesting almost the opposite.
> > Declarations should only have initialisers if the value is constant.
>
> Yup. This new rule sound like something taught to people in coding schools.
> But initializing everything defeats the compiler detection of uninitialized variables
> which is more useful for catching errors.
You'll also get:
Values being read the wrong side of locks.
Values being read early so requiring spilling to stack.
Next someone will be suggesting that all pointers should be checked
against NULL every time they are used.
David
^ permalink raw reply
* Re: 100% CPU load when generating traffic to destination network that nexthop is not reachable
From: Eric Dumazet @ 2017-08-15 16:57 UTC (permalink / raw)
To: Paweł Staszewski; +Cc: Linux Kernel Network Developers
In-Reply-To: <177465a0-aaba-4063-e451-ebbf46728b77@itcare.pl>
On Tue, 2017-08-15 at 18:30 +0200, Paweł Staszewski wrote:
> Hi
>
>
> Doing some tests i discovered that when traffic is send by pktgen to
> forwarding host where nexthop for destination network on forwarding
> router is not reachable i have 100% cpu on all cores and perf top show
> mostly:
>
> 77.19% [kernel] [k] queued_spin_lock_slowpath
> 10.20% [kernel] [k] acpi_processor_ffh_cstate_enter
> 1.41% [kernel] [k] queued_write_lock_slowpath
>
To the rescue (for us to help)
perf record -a -g sleep 10
perf report --stdio
^ permalink raw reply
* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-15 17:00 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Matthew Dawson, Network Development, Macieira, Thiago
In-Reply-To: <CAF=yD-Lacw9T7G0XZBdHZwEb6HgRuaBoyUN1oTvWixu4a0Fy6Q@mail.gmail.com>
> There is another difference between reading sk_peek_offset in the
> caller or in __skb_try_recv_from_queue. The latter is called repeatedly
> when it returns NULL. Each call can modify *off. I believe that it needs
> to restart with _off at sk->sk_peek_off each time, as it restarts from the
> head of the queue each time.
I made a mistake here. *off is not updated when returning NULL.
In that case, it is better to read sk_peek_offset once, than to read
it each time __skb_try_recv_from_queue is entered.
^ permalink raw reply
* RE: [PATCH v4 iproute2 2/7] rdma: Add dev object
From: David Laight @ 2017-08-15 17:00 UTC (permalink / raw)
To: 'Leon Romanovsky'
Cc: 'Stephen Hemminger', Doug Ledford,
linux-rdma@vger.kernel.org, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815164720.GV24282@mtr-leonro.local>
From: Leon Romanovsky
> Sent: 15 August 2017 17:47
> On Tue, Aug 15, 2017 at 04:23:11PM +0000, David Laight wrote:
> > From: Stephen Hemminger
> > > Sent: 15 August 2017 17:12
> > > On Tue, 15 Aug 2017 16:00:15 +0300
> > > Leon Romanovsky <leonro@mellanox.com> wrote:
> > >
> > > > +
> > > > +static const char *dev_caps_to_str(uint32_t idx)
> > > > +{
> > > > + uint64_t cap = 1 << idx;
> > > > +
> > > > + switch (cap) {
> > > > + case RDMA_DEV_RESIZE_MAX_WR: return "RESIZE_MAX_WR";
> > > > + case RDMA_DEV_BAD_PKEY_CNTR: return "BAD_PKEY_CNTR";
> > ...
> > > > + case RDMA_DEV_RAW_SCATTER_FCS: return "RAW_SCATTER_FCS";
> > > > + case RDMA_DEV_RDMA_NETDEV_OPA_VNIC: return "RDMA_NETDEV_OPA_VNIC";
> > > > + default: return "UNKNOWN";
> > > > + }
> > >
> > > Could this be a table in future versions?
> >
> > Potentially you could define the constants using some pre-processor
> > 'magic' that would create the table for you.
> > Something like (but not compiled):
> >
> > #define RDMA_DEV_FLAGS(x) \
> > x(RESIZE_MAX_WR, 0) \
> > x(BAD_PKEY_CNTR, 1) \
> > (continue for all the bits)
> >
> > #define RDMA_DEV_ENUM(name, bit_no) RDMA_DEV_##name = BIT(bit_no),
> > enum {RDMA_DEV_FLAGS(RDMA_DEV_ENUM)};
> > #undef RDMA_DEV_ENUM
> >
> > #define RDMA_DEV_NAMES(name, bit_no) [bit_no] = #name,
> > static const char rdma_dev_names[] = {RDMA_DEV_FLAGS(RDMA_DEV_NAMES)};
I missed the #undef
> >
>
> David,
>
> How should I handle "uknown" fields without names?
The function that accesses rdma_dev_names[] checks ARRAY_SIZE()
and for a NULL pointer.
WRT the other comment, the spare pointers consume far less space than
the code for your switch statement.
David
^ permalink raw reply
* Re: [PATCH net RESEND] PCI: fix oops when try to find Root Port for a PCI device
From: Bjorn Helgaas @ 2017-08-15 17:03 UTC (permalink / raw)
To: Ding Tianhong
Cc: leedom, ashok.raj, bhelgaas, werner, ganeshgr, asit.k.mallick,
patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw, l.stach, amira,
gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
catalin.marinas, will.deacon, mark.rutland, robin.murphy, davem,
alexander.duyck, eric.dumazet, linux-arm-kernel, netdev,
linux-pci, linux-kernel, linuxarm
In-Reply-To: <1502810688-12420-1-git-send-email-dingtianhong@huawei.com>
On Tue, Aug 15, 2017 at 11:24:48PM +0800, Ding Tianhong wrote:
> Eric report a oops when booting the system after applying
> the commit a99b646afa8a ("PCI: Disable PCIe Relaxed..."):
> ...
> It looks like the pci_find_pcie_root_port() was trying to
> find the Root Port for the PCI device which is the Root
> Port already, it will return NULL and trigger the problem,
> so check the highest_pcie_bridge to fix thie problem.
The problem was actually with a Root Complex Integrated Endpoint that
has no upstream PCIe device:
00:05.2 System peripheral: Intel Corporation Device 0e2a (rev 04)
Subsystem: Intel Corporation Device 0e2a
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
ExtTag- RBE- FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal+ Unsupported+
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
MaxPayload 128 bytes, MaxReadReq 128 bytes
> Fixes: a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")
This also
Fixes: c56d4450eb68 ("PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum")
which added pci_find_pcie_root_port(). Prior to this Relaxed Ordering
series, we only used pci_find_pcie_root_port() in a Chelsio quirk that
only applied to non-integrated endpoints, so we didn't trip over the
bug.
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> drivers/pci/pci.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index af0cc34..7e2022f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -522,7 +522,8 @@ struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
> bridge = pci_upstream_bridge(bridge);
> }
>
> - if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> + if (highest_pcie_bridge &&
> + pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> return NULL;
>
> return highest_pcie_bridge;
> --
I think structuring the fix as follows is a little more readable:
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af0cc3456dc1..587cd7623ed8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -522,10 +522,11 @@ struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
bridge = pci_upstream_bridge(bridge);
}
- if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
- return NULL;
+ if (highest_pcie_bridge &&
+ pci_pcie_type(highest_pcie_bridge) == PCI_EXP_TYPE_ROOT_PORT)
+ return highest_pcie_bridge;
- return highest_pcie_bridge;
+ return NULL;
}
EXPORT_SYMBOL(pci_find_pcie_root_port);
^ permalink raw reply related
* Re: [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile
From: Randy Dunlap @ 2017-08-15 17:10 UTC (permalink / raw)
To: mohamedalrshah, davem
Cc: netdev, torvalds, linux-kernel, Mohamed Othman, Borhanuddin Ali,
Zurina Hanapi
In-Reply-To: <20170815130806.25168-1-mohamed.a.alrshah@ieee.org>
On 08/15/2017 06:08 AM, mohamedalrshah wrote:
> diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> index 91a2557..474f72c 100644
> --- a/net/ipv4/Kconfig
> +++ b/net/ipv4/Kconfig
> @@ -677,6 +677,17 @@ config TCP_CONG_BBR
> bufferbloat, policers, or AQM schemes that do not provide a delay
> signal. It requires the fq ("Fair Queue") pacing packet scheduler.
>
> +config TCP_CONG_AGILESD
> + tristate "Agile-SD Congestion control"
> + default n
> + ---help---
> +
> + This is version 1.0 of Agile-SD TCP. It is a sender-side only.
It is a sender-side only <what>.
> + It contributes the Agility Factor (AF) to shorten the epoch time
> + and to make TCP independent from RTT. AF reduces the sensitivity
> + to packet losses, which in turn Agile-SD to achieve better throughput
in turn allows Agile-SD
> + over high-speed networks.
> +
Please drop the tab(s) and space(s) at the ends of lines.
> choice
> prompt "Default TCP congestion control"
> default DEFAULT_CUBIC
> @@ -713,6 +724,9 @@ choice
>
> config DEFAULT_BBR
> bool "BBR" if TCP_CONG_BBR=y
> +
> + config DEFAULT_AGILESD
Indent above with tab, not spaces.
> + bool "AGILESD" if TCP_CONG_AGILESD=y
>
> config DEFAULT_RENO
> bool "Reno"
> @@ -738,6 +752,7 @@ config DEFAULT_TCP_CONG
> default "dctcp" if DEFAULT_DCTCP
> default "cdg" if DEFAULT_CDG
> default "bbr" if DEFAULT_BBR
> + default "agilesd" if DEFAULT_AGILESD
Indent above with tab, not spaces.
> default "cubic"
>
> config TCP_MD5SIG
--
~Randy
^ permalink raw reply
* Re: pull-request: wireless-drivers 2017-08-15
From: David Miller @ 2017-08-15 17:19 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87d17x9ig5.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Tue, 15 Aug 2017 14:30:34 +0300
> more fixes to net tree for 4.13. More info in the signed tag below,
> please let me know if there are any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH 1/2] mlx4: remove unnecessary pci_set_drvdata()
From: Jason Gunthorpe @ 2017-08-15 17:21 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Zhu Yanjun, saeedm, matanb, netdev, linux-rdma, yuval.shaia
In-Reply-To: <20170815071336.GL24282@mtr-leonro.local>
On Tue, Aug 15, 2017 at 10:13:36AM +0300, Leon Romanovsky wrote:
> On Tue, Aug 15, 2017 at 02:33:05AM -0400, Zhu Yanjun wrote:
> > The driver core clears the driver data to NULL after device_release
> > or on probe failure. Thus, it is not necessary to manually clear the
> > device driver data to NULL.
> >
>
> It makes sense and I'm pretty sure that you are right, but I'm failing
> to find the function in device core which sets it to NULL. Can you help
> me and present the actual call stack to that code place?
http://elixir.free-electrons.com/linux/v4.13-rc1/source/drivers/base/dd.c#L840
The call to the remove callback is on line 833.
This is done after dropping devres, so you could allocate the drv data
inside a devm object and everything would unwind correctly.
In this case, the kfree is explicit, so I would advocate for still putting
the null near the kfree to minimize the time where a free'd pointer is
present - eg incase a devm callback or some other bug accidently
touches it.
Jason
^ 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