* Re: [PATCH net-next] net: phy: mscc: the extended page access register is 16 bits
From: David Miller @ 2018-07-30 16:43 UTC (permalink / raw)
To: quentin.schulz
Cc: andrew, f.fainelli, netdev, alexandre.belloni, linux-kernel,
thomas.petazzoni
In-Reply-To: <20180730125313.3405-1-quentin.schulz@bootlin.com>
From: Quentin Schulz <quentin.schulz@bootlin.com>
Date: Mon, 30 Jul 2018 14:53:13 +0200
> The Extended Page Access is a 16-bit register, so change the page
> parameter of vsc85xx_phy_page_set to a u16.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
Applied.
^ permalink raw reply
* your photos need this
From: Edward @ 2018-07-30 10:52 UTC (permalink / raw)
To: netdev
We are an image editing service provider having more than 10 years of
industry experience.
We use latest techniques for photo editing to meet international quality
standards.
We are committed to deliver reliable photo processing services.
Production of 1000+ images within single working day
24×7×365 photo editing services
On-demand image editing to meet rush job requirements
Service details:
Images Masking
Photo Clipping Path
Photo Cutout
Photo Enhancement
Vector Conversion
Photo Stitching Services
Fashion Photo Editing
Jewelry Retouching
Footwear Photo Editing
Furniture Photo Retouching
Wedding Photo Editing
Real Estate Photo Editing
Photo Restoration
We provide trials to evaluate our service quality to new clients.
Thanks,
Edward
^ permalink raw reply
* Re: [PATCH] xen-netfront: wait xenbus state change when load module manually
From: David Miller @ 2018-07-30 16:41 UTC (permalink / raw)
To: xiliang; +Cc: netdev, xen-devel, jgross, boris.ostrovsky, linux-kernel
In-Reply-To: <20180727095608.25210-1-xiliang@redhat.com>
From: Xiao Liang <xiliang@redhat.com>
Date: Fri, 27 Jul 2018 17:56:08 +0800
> When loading module manually, after call xenbus_switch_state to initializes
> the state of the netfront device, the driver state did not change so fast
> that may lead no dev created in latest kernel. This patch adds wait to make
> sure xenbus knows the driver is not in closed/unknown state.
...
> Signed-off-by: Xiao Liang <xiliang@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Alexander Duyck @ 2018-07-30 15:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Moshe Shemesh, Jiri Pirko, Jakub Kicinski, Eran Ben Elisha,
Saeed Mahameed, David S. Miller, netdev@vger.kernel.org,
linux-pci
In-Reply-To: <20180730140727.GJ173328@bhelgaas-glaptop.roam.corp.google.com>
On Mon, Jul 30, 2018 at 7:07 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Sun, Jul 29, 2018 at 03:00:28PM -0700, Alexander Duyck wrote:
>> On Sun, Jul 29, 2018 at 2:23 AM, Moshe Shemesh <moshes20.il@gmail.com> wrote:
>> > On Sat, Jul 28, 2018 at 7:06 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> >> On Thu, Jul 26, 2018 at 07:00:20AM -0700, Alexander Duyck wrote:
>> >> > On Thu, Jul 26, 2018 at 12:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> >> > > Thu, Jul 26, 2018 at 02:43:59AM CEST, jakub.kicinski@netronome.com
>> >> > > wrote:
>> >> > >>On Wed, 25 Jul 2018 08:23:26 -0700, Alexander Duyck wrote:
>> >> > >>> On Wed, Jul 25, 2018 at 5:31 AM, Eran Ben Elisha wrote:
>> >> > >>> > On 7/24/2018 10:51 PM, Jakub Kicinski wrote:
>> >> > >>> >>>> The devlink params haven't been upstream even for a full cycle
>> >> > >>> >>>> and
>> >> > >>> >>>> already you guys are starting to use them to configure standard
>> >> > >>> >>>> features like queuing.
>> >> > >>> >>>
>> >> > >>> >>> We developed the devlink params in order to support non-standard
>> >> > >>> >>> configuration only. And for non-standard, there are generic and
>> >> > >>> >>> vendor
>> >> > >>> >>> specific options.
>> >> > >>> >>
>> >> > >>> >> I thought it was developed for performing non-standard and
>> >> > >>> >> possibly
>> >> > >>> >> vendor specific configuration. Look at DEVLINK_PARAM_GENERIC_*
>> >> > >>> >> for
>> >> > >>> >> examples of well justified generic options for which we have no
>> >> > >>> >> other API. The vendor mlx4 options look fairly vendor specific
>> >> > >>> >> if you
>> >> > >>> >> ask me, too.
>> >> > >>> >>
>> >> > >>> >> Configuring queuing has an API. The question is it acceptable to
>> >> > >>> >> enter
>> >> > >>> >> into the risky territory of controlling offloads via devlink
>> >> > >>> >> parameters
>> >> > >>> >> or would we rather make vendors take the time and effort to model
>> >> > >>> >> things to (a subset) of existing APIs. The HW never fits the
>> >> > >>> >> APIs
>> >> > >>> >> perfectly.
>> >> > >>> >
>> >> > >>> > I understand what you meant here, I would like to highlight that
>> >> > >>> > this
>> >> > >>> > mechanism was not meant to handle SRIOV, Representors, etc.
>> >> > >>> > The vendor specific configuration suggested here is to handle a
>> >> > >>> > congestion
>> >> > >>> > state in Multi Host environment (which includes PF and multiple
>> >> > >>> > VFs per
>> >> > >>> > host), where one host is not aware to the other hosts, and each is
>> >> > >>> > running
>> >> > >>> > on its own pci/driver. It is a device working mode configuration.
>> >> > >>> >
>> >> > >>> > This couldn't fit into any existing API, thus creating this
>> >> > >>> > vendor specific
>> >> > >>> > unique API is needed.
>> >> > >>>
>> >> > >>> If we are just going to start creating devlink interfaces in for
>> >> > >>> every
>> >> > >>> one-off option a device wants to add why did we even bother with
>> >> > >>> trying to prevent drivers from using sysfs? This just feels like we
>> >> > >>> are back to the same arguments we had back in the day with it.
>> >> > >>>
>> >> > >>> I feel like the bigger question here is if devlink is how we are
>> >> > >>> going
>> >> > >>> to deal with all PCIe related features going forward, or should we
>> >> > >>> start looking at creating a new interface/tool for PCI/PCIe related
>> >> > >>> features? My concern is that we have already had features such as
>> >> > >>> DMA
>> >> > >>> Coalescing that didn't really fit into anything and now we are
>> >> > >>> starting to see other things related to DMA and PCIe bus credits.
>> >> > >>> I'm
>> >> > >>> wondering if we shouldn't start looking at a tool/interface to
>> >> > >>> configure all the PCIe related features such as interrupts, error
>> >> > >>> reporting, DMA configuration, power management, etc. Maybe we could
>> >> > >>> even look at sharing it across subsystems and include things like
>> >> > >>> storage, graphics, and other subsystems in the conversation.
>> >> > >>
>> >> > >>Agreed, for actual PCIe configuration (i.e. not ECN marking) we do
>> >> > >> need
>> >> > >>to build up an API. Sharing it across subsystems would be very cool!
>> >>
>> >> I read the thread (starting at [1], for anybody else coming in late)
>> >> and I see this has something to do with "configuring outbound PCIe
>> >> buffers", but I haven't seen the connection to PCIe protocol or
>> >> features, i.e., I can't connect this to anything in the PCIe spec.
>> >>
>> >> Can somebody help me understand how the PCI core is relevant? If
>> >> there's some connection with a feature defined by PCIe, or if it
>> >> affects the PCIe transaction protocol somehow, I'm definitely
>> >> interested in this. But if this only affects the data transferred
>> >> over PCIe, i.e., the data payloads of PCIe TLP packets, then I'm not
>> >> sure why the PCI core should care.
>> >>
>> >
>> >
>> > As you wrote, this is not a PCIe feature or affects the PCIe transaction
>> > protocol.
>> >
>> > Actually, due to hardware limitation in current device, we have enabled a
>> > workaround in hardware.
>> >
>> > This mode is proprietary and not relevant to other PCIe devices, thus is set
>> > using driver-specific parameter in devlink
>>
>> Essentially what this feature is doing is communicating the need for
>> PCIe back-pressure to the network fabric. So as the buffers on the
>> device start to fill because the device isn't able to get back PCIe
>> credits fast enough it will then start to send congestion
>> notifications to the network stack itself if I understand this
>> correctly.
>
> This sounds like a hook that allows the device to tell its driver
> about PCIe flow control credits, and the driver can pass that on to
> the network stack. IIUC, that would be a device-specific feature
> outside the scope of the PCI core.
>
>> For now there are no major conflicts, but when we start getting into
>> stuff like PCIe DMA coalescing, and on a more general basis just PCIe
>> active state power management that is going to start making things
>> more complicated going forward.
>
> We do support ASPM already in the PCI core, and we do have the
> pci_disable_link_state() interface, which is currently the only way
> drivers can influence it. There are several drivers that do their own
> ASPM configuration, but this is not safe because it's not coordinated
> with what the PCI core does. If/when drivers need more control, we
> should enhance the PCI core interfaces.
This is kind of what I was getting at. It would be useful to have an
interface of some sort so that drivers get notified when a user is
making changes to configuration space and I don't know if anything
like that exists now.
> I don't know what PCIe DMA coalescing means, so I can't comment on
> that.
There are devices, specifically network devices, that will hold off on
switching between either L0s or L1 and L0 by deferring DMA operations.
Basically the idea is supposed to be to hold off bringing the link up
for as long as possible in order to maximize power savings for the
ASPM state. This is something that has come up in the past, and I
don't know if there has been any interface determined for how to
handle this sort of configuration. Most of it occurs through MMIO.
>> I assume the devices we are talking about supporting this new feature
>> on either don't deal with ASPM or assume a quick turnaround to get out
>> of the lower power states? Otherwise that would definitely cause some
>> back-pressure buildups that would hurt performance.
>
> Devices can communicate the ASPM exit latency they can tolerate via
> the Device Capabilities register (PCIe r4.0, sec 76.5.3.3). Linux
> should be configuring ASPM to respect those device requirements.
>
> Bjorn
Right. But my point was something like ASPM will add extra complexity
to a feature such as what has been described here. My concern is that
I don't want us implementing stuff on a per-driver basis that is not
all that unique to the device. I don't really see the feature that was
described above as being something that will stay specific to this one
device for very long, especially if it provides added value. Basically
all it is doing is allowing exposing PCIe congestion management to
upper levels in the network stack. I don't even necessarily see it as
being networking specific as I would imagine there might be other
types of devices that could make use of knowing how many transactions
and such they could process at the same time.
- Alex
^ permalink raw reply
* Re: [PATCH net-next] liquidio: remove redundant function cn23xx_dump_iq_regs
From: David Miller @ 2018-07-30 16:27 UTC (permalink / raw)
To: yuehaibing
Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
linux-kernel, netdev
In-Reply-To: <20180730110341.1656-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 30 Jul 2018 19:03:41 +0800
> There are no in-tree callers of cn23xx_dump_iq_regs.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH 3/3] ptp_qoriq: convert to use module parameters for initialization
From: David Miller @ 2018-07-30 16:25 UTC (permalink / raw)
To: yangbo.lu
Cc: netdev, madalin.bucur, richardcochran, robh+dt, shawnguo,
devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel
In-Reply-To: <20180730100154.27906-3-yangbo.lu@nxp.com>
From: Yangbo Lu <yangbo.lu@nxp.com>
Date: Mon, 30 Jul 2018 18:01:54 +0800
> +static unsigned int cksel = DEFAULT_CKSEL;
> +module_param(cksel, uint, 0644);
> +MODULE_PARM_DESC(cksel, "Select reference clock");
> +
> +static unsigned int clk_src;
> +module_param(clk_src, uint, 0644);
> +MODULE_PARM_DESC(clk_src, "Reference clock frequency (if clocks property not provided in dts)");
> +
> +static unsigned int tmr_prsc = 2;
> +module_param(tmr_prsc, uint, 0644);
> +MODULE_PARM_DESC(tmr_prsc, "Output clock division/prescale factor");
> +
> +static unsigned int tmr_fiper1 = 1000000000;
> +module_param(tmr_fiper1, uint, 0644);
> +MODULE_PARM_DESC(tmr_fiper1, "Desired fixed interval pulse period (ns)");
> +
> +static unsigned int tmr_fiper2 = 100000;
> +module_param(tmr_fiper2, uint, 0644);
> +MODULE_PARM_DESC(tmr_fiper2, "Desired fixed interval pulse period (ns)");
Sorry, there is no way I am every applying something like this. Module
parameters are to be avoided at all costs.
And you don't need it here, you have DTS, please use it.
You are required to support the existing DTS cases, in order to
avoid breaking things, anyways.
^ permalink raw reply
* Hello
From: Mr. Kamalesh Ashu @ 2018-07-30 14:42 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 246 bytes --]
DEAR FRIENDI am(Mr.Kamalesh Ashu),With the business proposal deal of US(US$10.5 million) to transfer into your account, if you are interested get back to me for more detail at my E-mail(ashukamalesh4356@gmail.com)Best Regard
Mr. Kamalesh Ashu
[-- Attachment #2: Type: text/html, Size: 1816 bytes --]
^ permalink raw reply
* [PATCH net-next 5/5] selftests: forwarding: Test mirror-to-gretap w/ UL team LACP
From: Petr Machata @ 2018-07-30 14:40 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
In-Reply-To: <cover.1532961216.git.petrm@mellanox.com>
This tests mirror-to-gretap when an underlay packet path includes a team
device which is not in loadbalance mode, but in LACP mode. The test
manipulates LAG membership to achieve changes in txability, thus making
sure that a driver that offloads mirror-to-gretap doesn't just consider
upness of a device.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
.../net/forwarding/mirror_gre_lag_lacp.sh | 285 +++++++++++++++++++++
1 file changed, 285 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh
diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh b/tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh
new file mode 100755
index 000000000000..9edf4cb104a8
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh
@@ -0,0 +1,285 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test for "tc action mirred egress mirror" when the underlay route points at a
+# team device.
+#
+# +----------------------+ +----------------------+
+# | H1 | | H2 |
+# | + $h1.333 | | $h1.555 + |
+# | | 192.0.2.1/28 | | 192.0.2.18/28 | |
+# +----|-----------------+ +----------------|-----+
+# | $h1 |
+# +---------------------------------+------------------------------+
+# |
+# +--------------------------------------|------------------------------------+
+# | SW o---> mirror |
+# | | |
+# | +----------------------------------+------------------------------+ |
+# | | $swp1 | |
+# | + $swp1.333 $swp1.555 + |
+# | 192.0.2.2/28 192.0.2.17/28 |
+# | |
+# | |
+# | + gt4 (gretap) ,-> + lag1 (team) |
+# | loc=192.0.2.129 | | 192.0.2.129/28 |
+# | rem=192.0.2.130 --' | |
+# | ttl=100 | |
+# | tos=inherit | |
+# | _____________________|______________________ |
+# | / \ |
+# | / \ |
+# | + $swp3 + $swp4 |
+# +---|------------------------------------------------|----------------------+
+# | |
+# +---|------------------------------------------------|----------------------+
+# | + $h3 + $h4 H3 |
+# | \ / |
+# | \____________________________________________/ |
+# | | |
+# | + lag2 (team) |
+# | 192.0.2.130/28 |
+# | |
+# +---------------------------------------------------------------------------+
+
+ALL_TESTS="
+ test_mirror_gretap_first
+ test_mirror_gretap_second
+"
+
+NUM_NETIFS=6
+source lib.sh
+source mirror_lib.sh
+source mirror_gre_lib.sh
+
+require_command $ARPING
+
+vlan_host_create()
+{
+ local if_name=$1; shift
+ local vid=$1; shift
+ local vrf_name=$1; shift
+ local ips=("${@}")
+
+ vrf_create $vrf_name
+ ip link set dev $vrf_name up
+ vlan_create $if_name $vid $vrf_name "${ips[@]}"
+}
+
+vlan_host_destroy()
+{
+ local if_name=$1; shift
+ local vid=$1; shift
+ local vrf_name=$1; shift
+
+ vlan_destroy $if_name $vid
+ ip link set dev $vrf_name down
+ vrf_destroy $vrf_name
+}
+
+h1_create()
+{
+ vlan_host_create $h1 333 vrf-h1 192.0.2.1/28
+ ip -4 route add 192.0.2.16/28 vrf vrf-h1 nexthop via 192.0.2.2
+}
+
+h1_destroy()
+{
+ ip -4 route del 192.0.2.16/28 vrf vrf-h1
+ vlan_host_destroy $h1 333 vrf-h1
+}
+
+h2_create()
+{
+ vlan_host_create $h1 555 vrf-h2 192.0.2.18/28
+ ip -4 route add 192.0.2.0/28 vrf vrf-h2 nexthop via 192.0.2.17
+}
+
+h2_destroy()
+{
+ ip -4 route del 192.0.2.0/28 vrf vrf-h2
+ vlan_host_destroy $h1 555 vrf-h2
+}
+
+h3_create_team()
+{
+ team_create lag2 lacp $h3 $h4
+ __simple_if_init lag2 vrf-h3 192.0.2.130/32
+ ip -4 route add vrf vrf-h3 192.0.2.129/32 dev lag2
+}
+
+h3_destroy_team()
+{
+ ip -4 route del vrf vrf-h3 192.0.2.129/32 dev lag2
+ __simple_if_fini lag2 192.0.2.130/32
+ team_destroy lag2
+
+ ip link set dev $h3 down
+ ip link set dev $h4 down
+}
+
+h3_create()
+{
+ vrf_create vrf-h3
+ ip link set dev vrf-h3 up
+ tc qdisc add dev $h3 clsact
+ tc qdisc add dev $h4 clsact
+ h3_create_team
+}
+
+h3_destroy()
+{
+ h3_destroy_team
+ tc qdisc del dev $h4 clsact
+ tc qdisc del dev $h3 clsact
+ ip link set dev vrf-h3 down
+ vrf_destroy vrf-h3
+}
+
+switch_create()
+{
+ ip link set dev $swp1 up
+ tc qdisc add dev $swp1 clsact
+ vlan_create $swp1 333 "" 192.0.2.2/28
+ vlan_create $swp1 555 "" 192.0.2.17/28
+
+ tunnel_create gt4 gretap 192.0.2.129 192.0.2.130 \
+ ttl 100 tos inherit
+
+ ip link set dev $swp3 up
+ ip link set dev $swp4 up
+ team_create lag1 lacp $swp3 $swp4
+ __addr_add_del lag1 add 192.0.2.129/32
+ ip -4 route add 192.0.2.130/32 dev lag1
+}
+
+switch_destroy()
+{
+ ip -4 route del 192.0.2.130/32 dev lag1
+ __addr_add_del lag1 del 192.0.2.129/32
+ team_destroy lag1
+
+ ip link set dev $swp4 down
+ ip link set dev $swp3 down
+
+ tunnel_destroy gt4
+
+ vlan_destroy $swp1 555
+ vlan_destroy $swp1 333
+ tc qdisc del dev $swp1 clsact
+ ip link set dev $swp1 down
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ swp3=${NETIFS[p3]}
+ h3=${NETIFS[p4]}
+
+ swp4=${NETIFS[p5]}
+ h4=${NETIFS[p6]}
+
+ vrf_prepare
+
+ ip link set dev $h1 up
+ h1_create
+ h2_create
+ h3_create
+ switch_create
+
+ trap_install $h3 ingress
+ trap_install $h4 ingress
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ trap_uninstall $h4 ingress
+ trap_uninstall $h3 ingress
+
+ switch_destroy
+ h3_destroy
+ h2_destroy
+ h1_destroy
+ ip link set dev $h1 down
+
+ vrf_cleanup
+}
+
+test_lag_slave()
+{
+ local up_dev=$1; shift
+ local down_dev=$1; shift
+ local what=$1; shift
+
+ RET=0
+
+ mirror_install $swp1 ingress gt4 \
+ "proto 802.1q flower vlan_id 333 $tcflags"
+
+ # Move $down_dev away from the team. That will prompt change in
+ # txability of the connected device, without changing its upness. The
+ # driver should notice the txability change and move the traffic to the
+ # other slave.
+ ip link set dev $down_dev nomaster
+ sleep 2
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $up_dev 1 10
+
+ # Test lack of connectivity when neither slave is txable.
+ ip link set dev $up_dev nomaster
+ sleep 2
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $h3 1 0
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $h4 1 0
+ mirror_uninstall $swp1 ingress
+
+ # Recreate H3's team device, because mlxsw, which this test is
+ # predominantly mean to test, requires a bottom-up construction and
+ # doesn't allow enslavement to a device that already has an upper.
+ h3_destroy_team
+ h3_create_team
+ # Wait for ${h,swp}{3,4}.
+ setup_wait
+
+ log_test "$what ($tcflags)"
+}
+
+test_mirror_gretap_first()
+{
+ test_lag_slave $h3 $h4 "mirror to gretap: LAG first slave"
+}
+
+test_mirror_gretap_second()
+{
+ test_lag_slave $h4 $h3 "mirror to gretap: LAG second slave"
+}
+
+test_all()
+{
+ slow_path_trap_install $swp1 ingress
+ slow_path_trap_install $swp1 egress
+
+ tests_run
+
+ slow_path_trap_uninstall $swp1 egress
+ slow_path_trap_uninstall $swp1 ingress
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tcflags="skip_hw"
+test_all
+
+if ! tc_offload_check; then
+ echo "WARN: Could not test offloaded functionality"
+else
+ tcflags="skip_sw"
+ test_all
+fi
+
+exit $EXIT_STATUS
--
2.4.11
^ permalink raw reply related
* [PATCH net-next 4/5] selftests: forwarding: Test mirror-to-gretap w/ UL team
From: Petr Machata @ 2018-07-30 14:40 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
In-Reply-To: <cover.1532961216.git.petrm@mellanox.com>
Test for "tc action mirred egress mirror" that mirrors to gretap when
the underlay route points at a VLAN-aware bridge (802.1q), and the
traffic egresses the bridge through a team device. Test upping and
downing individual team device slaves and verify the traffic flows as
expected.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
.../net/forwarding/mirror_gre_bridge_1q_lag.sh | 283 +++++++++++++++++++++
1 file changed, 283 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
new file mode 100755
index 000000000000..61844caf671e
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
@@ -0,0 +1,283 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test for "tc action mirred egress mirror" when the underlay route points at a
+# bridge device with vlan filtering (802.1q), and the egress device is a team
+# device.
+#
+# +----------------------+ +----------------------+
+# | H1 | | H2 |
+# | + $h1.333 | | $h1.555 + |
+# | | 192.0.2.1/28 | | 192.0.2.18/28 | |
+# +-----|----------------+ +----------------|-----+
+# | $h1 |
+# +--------------------------------+------------------------------+
+# |
+# +--------------------------------------|------------------------------------+
+# | SW o---> mirror |
+# | | |
+# | +--------------------------------+------------------------------+ |
+# | | $swp1 | |
+# | + $swp1.333 $swp1.555 + |
+# | 192.0.2.2/28 192.0.2.17/28 |
+# | |
+# | +-----------------------------------------------------------------------+ |
+# | | BR1 (802.1q) | |
+# | | + lag (team) 192.0.2.129/28 | |
+# | | / \ 2001:db8:2::1/64 | |
+# | +---/---\---------------------------------------------------------------+ |
+# | / \ ^ |
+# | | \ + gt4 (gretap) | |
+# | | \ loc=192.0.2.129 | |
+# | | \ rem=192.0.2.130 -+ |
+# | | \ ttl=100 |
+# | | \ tos=inherit |
+# | | \ |
+# | | \_________________________________ |
+# | | \ |
+# | + $swp3 + $swp4 |
+# +---|------------------------------------------------|----------------------+
+# | |
+# +---|----------------------+ +---|----------------------+
+# | + $h3 H3 | | + $h4 H4 |
+# | 192.0.2.130/28 | | 192.0.2.130/28 |
+# | 2001:db8:2::2/64 | | 2001:db8:2::2/64 |
+# +--------------------------+ +--------------------------+
+
+ALL_TESTS="
+ test_mirror_gretap_first
+ test_mirror_gretap_second
+"
+
+NUM_NETIFS=6
+source lib.sh
+source mirror_lib.sh
+source mirror_gre_lib.sh
+
+require_command $ARPING
+
+vlan_host_create()
+{
+ local if_name=$1; shift
+ local vid=$1; shift
+ local vrf_name=$1; shift
+ local ips=("${@}")
+
+ vrf_create $vrf_name
+ ip link set dev $vrf_name up
+ vlan_create $if_name $vid $vrf_name "${ips[@]}"
+}
+
+vlan_host_destroy()
+{
+ local if_name=$1; shift
+ local vid=$1; shift
+ local vrf_name=$1; shift
+
+ vlan_destroy $if_name $vid
+ ip link set dev $vrf_name down
+ vrf_destroy $vrf_name
+}
+
+h1_create()
+{
+ vlan_host_create $h1 333 vrf-h1 192.0.2.1/28
+ ip -4 route add 192.0.2.16/28 vrf vrf-h1 nexthop via 192.0.2.2
+}
+
+h1_destroy()
+{
+ ip -4 route del 192.0.2.16/28 vrf vrf-h1
+ vlan_host_destroy $h1 333 vrf-h1
+}
+
+h2_create()
+{
+ vlan_host_create $h1 555 vrf-h2 192.0.2.18/28
+ ip -4 route add 192.0.2.0/28 vrf vrf-h2 nexthop via 192.0.2.17
+}
+
+h2_destroy()
+{
+ ip -4 route del 192.0.2.0/28 vrf vrf-h2
+ vlan_host_destroy $h1 555 vrf-h2
+}
+
+h3_create()
+{
+ simple_if_init $h3 192.0.2.130/28
+ tc qdisc add dev $h3 clsact
+}
+
+h3_destroy()
+{
+ tc qdisc del dev $h3 clsact
+ simple_if_fini $h3 192.0.2.130/28
+}
+
+h4_create()
+{
+ simple_if_init $h4 192.0.2.130/28
+ tc qdisc add dev $h4 clsact
+}
+
+h4_destroy()
+{
+ tc qdisc del dev $h4 clsact
+ simple_if_fini $h4 192.0.2.130/28
+}
+
+switch_create()
+{
+ ip link set dev $swp1 up
+ tc qdisc add dev $swp1 clsact
+ vlan_create $swp1 333 "" 192.0.2.2/28
+ vlan_create $swp1 555 "" 192.0.2.17/28
+
+ tunnel_create gt4 gretap 192.0.2.129 192.0.2.130 \
+ ttl 100 tos inherit
+
+ ip link set dev $swp3 up
+ ip link set dev $swp4 up
+
+ ip link add name br1 type bridge vlan_filtering 1
+ ip link set dev br1 up
+ __addr_add_del br1 add 192.0.2.129/32
+ ip -4 route add 192.0.2.130/32 dev br1
+
+ team_create lag loadbalance $swp3 $swp4
+ ip link set dev lag master br1
+}
+
+switch_destroy()
+{
+ ip link set dev lag nomaster
+ team_destroy lag
+
+ ip -4 route del 192.0.2.130/32 dev br1
+ __addr_add_del br1 del 192.0.2.129/32
+ ip link set dev br1 down
+ ip link del dev br1
+
+ ip link set dev $swp4 down
+ ip link set dev $swp3 down
+
+ tunnel_destroy gt4
+
+ vlan_destroy $swp1 555
+ vlan_destroy $swp1 333
+ tc qdisc del dev $swp1 clsact
+ ip link set dev $swp1 down
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ swp3=${NETIFS[p3]}
+ h3=${NETIFS[p4]}
+
+ swp4=${NETIFS[p5]}
+ h4=${NETIFS[p6]}
+
+ vrf_prepare
+
+ ip link set dev $h1 up
+ h1_create
+ h2_create
+ h3_create
+ h4_create
+ switch_create
+
+ trap_install $h3 ingress
+ trap_install $h4 ingress
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ trap_uninstall $h4 ingress
+ trap_uninstall $h3 ingress
+
+ switch_destroy
+ h4_destroy
+ h3_destroy
+ h2_destroy
+ h1_destroy
+ ip link set dev $h1 down
+
+ vrf_cleanup
+}
+
+test_lag_slave()
+{
+ local host_dev=$1; shift
+ local up_dev=$1; shift
+ local down_dev=$1; shift
+ local what=$1; shift
+
+ RET=0
+
+ mirror_install $swp1 ingress gt4 \
+ "proto 802.1q flower vlan_id 333 $tcflags"
+
+ # Test connectivity through $up_dev when $down_dev is set down.
+ ip link set dev $down_dev down
+ setup_wait_dev $up_dev
+ setup_wait_dev $host_dev
+ $ARPING -I br1 192.0.2.130 -qfc 1
+ sleep 2
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $host_dev 1 10
+
+ # Test lack of connectivity when both slaves are down.
+ ip link set dev $up_dev down
+ sleep 2
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $h3 1 0
+ mirror_test vrf-h1 192.0.2.1 192.0.2.18 $h4 1 0
+
+ ip link set dev $up_dev up
+ ip link set dev $down_dev up
+ mirror_uninstall $swp1 ingress
+
+ log_test "$what ($tcflags)"
+}
+
+test_mirror_gretap_first()
+{
+ test_lag_slave $h3 $swp3 $swp4 "mirror to gretap: LAG first slave"
+}
+
+test_mirror_gretap_second()
+{
+ test_lag_slave $h4 $swp4 $swp3 "mirror to gretap: LAG second slave"
+}
+
+test_all()
+{
+ slow_path_trap_install $swp1 ingress
+ slow_path_trap_install $swp1 egress
+
+ tests_run
+
+ slow_path_trap_uninstall $swp1 egress
+ slow_path_trap_uninstall $swp1 ingress
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tcflags="skip_hw"
+test_all
+
+if ! tc_offload_check; then
+ echo "WARN: Could not test offloaded functionality"
+else
+ tcflags="skip_sw"
+ test_all
+fi
+
+exit $EXIT_STATUS
--
2.4.11
^ permalink raw reply related
* [PATCH net-next 3/5] selftests: forwarding: Introduce $ARPING
From: Petr Machata @ 2018-07-30 14:39 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
In-Reply-To: <cover.1532961216.git.petrm@mellanox.com>
Instead of relying on "arping" being installed everywhere under that
name, introduce a variable $ARPING like the other tools do.
Convert an existing test, mirror_gre_vlan_bridge_1q.sh to
require_command $ARPING and then invoke arping through the variable.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 1 +
tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 3a4eba4f0d08..843a6715924f 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -8,6 +8,7 @@
PING=${PING:=ping}
PING6=${PING6:=ping6}
MZ=${MZ:=mausezahn}
+ARPING=${ARPING:=arping}
TEAMD=${TEAMD:=teamd}
WAIT_TIME=${WAIT_TIME:=5}
PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
index d3e75bb6a2d8..204b25f13934 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
@@ -28,6 +28,8 @@ source mirror_lib.sh
source mirror_gre_lib.sh
source mirror_gre_topo_lib.sh
+require_command $ARPING
+
setup_prepare()
{
h1=${NETIFS[p1]}
@@ -149,7 +151,7 @@ test_span_gre_forbidden_egress()
bridge vlan add dev $swp3 vid 555
# Re-prime FDB
- arping -I br1.555 192.0.2.130 -fqc 1
+ $ARPING -I br1.555 192.0.2.130 -fqc 1
sleep 1
quick_test_span_gre_dir $tundev ingress
@@ -223,7 +225,7 @@ test_span_gre_fdb_roaming()
bridge fdb del dev $swp2 $h3mac vlan 555 master
# Re-prime FDB
- arping -I br1.555 192.0.2.130 -fqc 1
+ $ARPING -I br1.555 192.0.2.130 -fqc 1
sleep 1
quick_test_span_gre_dir $tundev ingress
--
2.4.11
^ permalink raw reply related
* [PATCH net-next 2/5] selftests: forwarding: lib: Support team devices
From: Petr Machata @ 2018-07-30 14:39 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
In-Reply-To: <cover.1532961216.git.petrm@mellanox.com>
Add team_create() and team_destroy() to manage team netdevices.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 81e36157bf16..3a4eba4f0d08 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -8,6 +8,7 @@
PING=${PING:=ping}
PING6=${PING6:=ping6}
MZ=${MZ:=mausezahn}
+TEAMD=${TEAMD:=teamd}
WAIT_TIME=${WAIT_TIME:=5}
PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
PAUSE_ON_CLEANUP=${PAUSE_ON_CLEANUP:=no}
@@ -425,6 +426,28 @@ vlan_destroy()
ip link del dev $name
}
+team_create()
+{
+ local if_name=$1; shift
+ local mode=$1; shift
+
+ require_command $TEAMD
+ $TEAMD -t $if_name -d -c '{"runner": {"name": "'$mode'"}}'
+ for slave in "$@"; do
+ ip link set dev $slave down
+ ip link set dev $slave master $if_name
+ ip link set dev $slave up
+ done
+ ip link set dev $if_name up
+}
+
+team_destroy()
+{
+ local if_name=$1; shift
+
+ $TEAMD -t $if_name -k
+}
+
master_name_get()
{
local if_name=$1
--
2.4.11
^ permalink raw reply related
* [PATCH net-next 1/5] selftests: forwarding: lib: Add require_command()
From: Petr Machata @ 2018-07-30 14:39 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
In-Reply-To: <cover.1532961216.git.petrm@mellanox.com>
The logic for testing whether a certain command is available is used
several times in the current code base. The tests in follow-up patches
add more requirements like that.
Therefore extract the logic into a named function, require_command(),
that can be used directly from lib.sh as well as from any test that
wishes to declare dependence on some command.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 158d59ffee40..81e36157bf16 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -62,15 +62,18 @@ if [[ "$CHECK_TC" = "yes" ]]; then
check_tc_version
fi
-if [[ ! -x "$(command -v jq)" ]]; then
- echo "SKIP: jq not installed"
- exit 1
-fi
+require_command()
+{
+ local cmd=$1; shift
-if [[ ! -x "$(command -v $MZ)" ]]; then
- echo "SKIP: $MZ not installed"
- exit 1
-fi
+ if [[ ! -x "$(command -v "$cmd")" ]]; then
+ echo "SKIP: $cmd not installed"
+ exit 1
+ fi
+}
+
+require_command jq
+require_command $MZ
if [[ ! -v NUM_NETIFS ]]; then
echo "SKIP: importer does not define \"NUM_NETIFS\""
--
2.4.11
^ permalink raw reply related
* [PATCH net-next 0/5] A test for mirror-to-gretap with team in UL packet path
From: Petr Machata @ 2018-07-30 14:37 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah
This patchset adds a test for "tc action mirred mirror" where the
mirrored-to device is a gretap, and underlay path contains a team
device.
In patch #1 require_command() is added, which should henceforth be used
to declare dependence on a certain tool.
In patch #2, two new functions, team_create() and team_destroy(), are
added to lib.sh.
The newly-added test uses arping, which isn't necessarily available.
Therefore patch #3 introduces $ARPING, and a preexisting test is fixed
to require_command $ARPING.
In patches #4 and #5, two new tests are added. In both cases, a team
device is on egress path of a mirrored packet in a mirror-to-gretap
scenario. In the first one, the team device is in loadbalance mode, in
the second one it's in lacp mode. (The difference in modes necessitates
a different testing strategy, hence two test cases instead of just
parameterizing one.)
Petr Machata (5):
selftests: forwarding: lib: Add require_command()
selftests: forwarding: lib: Support team devices
selftests: forwarding: Introduce $ARPING
selftests: forwarding: Test mirror-to-gretap w/ UL team
selftests: forwarding: Test mirror-to-gretap w/ UL team LACP
tools/testing/selftests/net/forwarding/lib.sh | 43 +++-
.../net/forwarding/mirror_gre_bridge_1q_lag.sh | 283 ++++++++++++++++++++
.../net/forwarding/mirror_gre_lag_lacp.sh | 285 +++++++++++++++++++++
.../net/forwarding/mirror_gre_vlan_bridge_1q.sh | 6 +-
4 files changed, 607 insertions(+), 10 deletions(-)
create mode 100755 tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
create mode 100755 tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh
--
2.4.11
^ permalink raw reply
* Re: [net-next PATCH] net: ipv4: fix listify ip_rcv_finish in case of forwarding
From: Or Gerlitz @ 2018-07-30 14:38 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Saeed Mahameed, Edward Cree, netdev@vger.kernel.org
In-Reply-To: <20180711220649.266b071a@redhat.com>
On Wed, Jul 11, 2018 at 11:06 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Wed, 11 Jul 2018 19:05:20 +0000
> Saeed Mahameed <saeedm@mellanox.com> wrote:
>
>> On Wed, 2018-07-11 at 17:01 +0200, Jesper Dangaard Brouer wrote:
>> > Only driver sfc actually uses this, but I don't have this NIC, so I
>> > tested this on mlx5, with my own changes to make it use
>> > netif_receive_skb_list(),
>> > but I'm not ready to upstream the mlx5 driver change yet.
>>
>>
>> Thanks Jesper for sharing this, should we look forward to those patches
>> or do you want us to implement them ?
>
> Well, I would prefer you to implement those. I just did a quick
> implementation (its trivially easy) so I have something to benchmark
> with. The performance boost is quite impressive!
>
> One reason I didn't "just" send a patch, is that Edward so-fare only
> implemented netif_receive_skb_list() and not napi_gro_receive_list().
> And your driver uses napi_gro_receive(). This sort-of disables GRO for
> your driver, which is not a choice I can make. Interestingly I get
But we can apply such patch on the mlx5 code path which comes
into play when GRO is disabled, right? or there's just one code path
there. Can you send your patch as RFC to the list for the purpose of
experimentation?
> around the same netperf TCP_STREAM performance. I assume we can get
> even better perf if we "listify" napi_gro_receive.
^ permalink raw reply
* Re: socket poll related cleanups v2
From: David Miller @ 2018-07-30 16:13 UTC (permalink / raw)
To: hch; +Cc: netdev, linux-kernel
In-Reply-To: <20180730074213.16832-1-hch@lst.de>
From: Christoph Hellwig <hch@lst.de>
Date: Mon, 30 Jul 2018 09:42:09 +0200
> A couple of cleanups I stumbled upon when studying the networking
> poll code.
>
> Changes since v1:
> - drop a dispute patch from this series (to be sent separately)
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next v5 1/4] net/sched: user-space can't set unknown tcfa_action values
From: Jiri Pirko @ 2018-07-30 14:21 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Paolo Abeni, netdev, Cong Wang, Daniel Borkmann,
Marcelo Ricardo Leitner, Eyal Birger, David S. Miller
In-Reply-To: <f455f493-c106-aa93-b92a-2aa94c969d31@mojatatu.com>
Mon, Jul 30, 2018 at 04:03:50PM CEST, jhs@mojatatu.com wrote:
>On 30/07/18 08:30 AM, Paolo Abeni wrote:
>> }
>> + if (!tcf_action_valid(a->tcfa_action)) {
>> + NL_SET_ERR_MSG(extack, "invalid action value, using TC_ACT_UNSPEC instead");
>> + a->tcfa_action = TC_ACT_UNSPEC;
>> + }
>> +
>> return a;
>
>
>I think it would make a lot more sense to just reject the entry than
>changing it underneath the user to a default value. Least element of
>suprise.
It might break existing user who is incorrectly doing it. But I'm okay
with both solutions.
>
>cheers,
>jamal
^ permalink raw reply
* [PATCH net] net/ipv6: fix metrics leak
From: Sabrina Dubroca @ 2018-07-30 14:23 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, David Ahern
Since commit d4ead6b34b67 ("net/ipv6: move metrics from dst to
rt6_info"), ipv6 metrics are shared and refcounted. rt6_set_from()
assigns the rt->from pointer and increases the refcount on from's
metrics. This reference is never released.
Introduce the fib6_metrics_release() helper and use it to release the
metrics.
Fixes: d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/ipv6/ip6_fib.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index d212738e9d10..211a2d437b56 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -167,11 +167,22 @@ struct fib6_info *fib6_info_alloc(gfp_t gfp_flags)
return f6i;
}
+static void fib6_metrics_release(struct fib6_info *f6i)
+{
+ struct dst_metrics *m;
+
+ if (!f6i)
+ return;
+
+ m = f6i->fib6_metrics;
+ if (m != &dst_default_metrics && refcount_dec_and_test(&m->refcnt))
+ kfree(m);
+}
+
void fib6_info_destroy_rcu(struct rcu_head *head)
{
struct fib6_info *f6i = container_of(head, struct fib6_info, rcu);
struct rt6_exception_bucket *bucket;
- struct dst_metrics *m;
WARN_ON(f6i->fib6_node);
@@ -201,9 +212,7 @@ void fib6_info_destroy_rcu(struct rcu_head *head)
if (f6i->fib6_nh.nh_dev)
dev_put(f6i->fib6_nh.nh_dev);
- m = f6i->fib6_metrics;
- if (m != &dst_default_metrics && refcount_dec_and_test(&m->refcnt))
- kfree(m);
+ fib6_metrics_release(f6i);
kfree(f6i);
}
@@ -887,6 +896,7 @@ static void fib6_drop_pcpu_from(struct fib6_info *f6i,
from = rcu_dereference_protected(pcpu_rt->from,
lockdep_is_held(&table->tb6_lock));
+ fib6_metrics_release(from);
rcu_assign_pointer(pcpu_rt->from, NULL);
fib6_info_release(from);
}
--
2.18.0
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 09/17] ethtool: implement GET_DRVINFO message
From: Andrew Lunn @ 2018-07-30 15:48 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, linux-kernel, Jiri Pirko, David Miller, Florian Fainelli,
Roopa Prabhu, Jakub Kicinski, John W. Linville
In-Reply-To: <20180730144644.r3utyf4toqkjcxwd@unicorn.suse.cz>
> This is interesting. It would mean current (ioctl) ethtool approach with
> string set may not work correctly either.
Hi Michal
For the statistics, it is a bit of a corner case. One of the Ethernet
switches in DSA can have two different PHYs linked to one MAC. One PHY
is built in, the second is connected via a SERDES interface. Which
every gets link first is used. However, the SERDES interface has
additional statistics counters. So if the SERDES is in use, we return
more statistics. If somebody was to plug in the cable at just the
wrong/right time, the count of statistics could be different to the
number of statistics.
Another corner case i can think of. Some drivers return statistics per
queue. And there is an ioctl to change the number of queues....
I could also imaging tests being similar. There are more loopback
tests you can do with a SERDES which you cannot do with a built in
PHY. But so far, i've not seen anything like that.
Andrew
^ permalink raw reply
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Bjorn Helgaas @ 2018-07-30 14:07 UTC (permalink / raw)
To: Alexander Duyck
Cc: Moshe Shemesh, Jiri Pirko, Jakub Kicinski, Eran Ben Elisha,
Saeed Mahameed, David S. Miller, netdev@vger.kernel.org,
linux-pci
In-Reply-To: <CAKgT0UfkBQKqy87BcnQvfzrbonR5rm9cd3Q2dzsyR04vY-+FWg@mail.gmail.com>
On Sun, Jul 29, 2018 at 03:00:28PM -0700, Alexander Duyck wrote:
> On Sun, Jul 29, 2018 at 2:23 AM, Moshe Shemesh <moshes20.il@gmail.com> wrote:
> > On Sat, Jul 28, 2018 at 7:06 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> On Thu, Jul 26, 2018 at 07:00:20AM -0700, Alexander Duyck wrote:
> >> > On Thu, Jul 26, 2018 at 12:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> >> > > Thu, Jul 26, 2018 at 02:43:59AM CEST, jakub.kicinski@netronome.com
> >> > > wrote:
> >> > >>On Wed, 25 Jul 2018 08:23:26 -0700, Alexander Duyck wrote:
> >> > >>> On Wed, Jul 25, 2018 at 5:31 AM, Eran Ben Elisha wrote:
> >> > >>> > On 7/24/2018 10:51 PM, Jakub Kicinski wrote:
> >> > >>> >>>> The devlink params haven't been upstream even for a full cycle
> >> > >>> >>>> and
> >> > >>> >>>> already you guys are starting to use them to configure standard
> >> > >>> >>>> features like queuing.
> >> > >>> >>>
> >> > >>> >>> We developed the devlink params in order to support non-standard
> >> > >>> >>> configuration only. And for non-standard, there are generic and
> >> > >>> >>> vendor
> >> > >>> >>> specific options.
> >> > >>> >>
> >> > >>> >> I thought it was developed for performing non-standard and
> >> > >>> >> possibly
> >> > >>> >> vendor specific configuration. Look at DEVLINK_PARAM_GENERIC_*
> >> > >>> >> for
> >> > >>> >> examples of well justified generic options for which we have no
> >> > >>> >> other API. The vendor mlx4 options look fairly vendor specific
> >> > >>> >> if you
> >> > >>> >> ask me, too.
> >> > >>> >>
> >> > >>> >> Configuring queuing has an API. The question is it acceptable to
> >> > >>> >> enter
> >> > >>> >> into the risky territory of controlling offloads via devlink
> >> > >>> >> parameters
> >> > >>> >> or would we rather make vendors take the time and effort to model
> >> > >>> >> things to (a subset) of existing APIs. The HW never fits the
> >> > >>> >> APIs
> >> > >>> >> perfectly.
> >> > >>> >
> >> > >>> > I understand what you meant here, I would like to highlight that
> >> > >>> > this
> >> > >>> > mechanism was not meant to handle SRIOV, Representors, etc.
> >> > >>> > The vendor specific configuration suggested here is to handle a
> >> > >>> > congestion
> >> > >>> > state in Multi Host environment (which includes PF and multiple
> >> > >>> > VFs per
> >> > >>> > host), where one host is not aware to the other hosts, and each is
> >> > >>> > running
> >> > >>> > on its own pci/driver. It is a device working mode configuration.
> >> > >>> >
> >> > >>> > This couldn't fit into any existing API, thus creating this
> >> > >>> > vendor specific
> >> > >>> > unique API is needed.
> >> > >>>
> >> > >>> If we are just going to start creating devlink interfaces in for
> >> > >>> every
> >> > >>> one-off option a device wants to add why did we even bother with
> >> > >>> trying to prevent drivers from using sysfs? This just feels like we
> >> > >>> are back to the same arguments we had back in the day with it.
> >> > >>>
> >> > >>> I feel like the bigger question here is if devlink is how we are
> >> > >>> going
> >> > >>> to deal with all PCIe related features going forward, or should we
> >> > >>> start looking at creating a new interface/tool for PCI/PCIe related
> >> > >>> features? My concern is that we have already had features such as
> >> > >>> DMA
> >> > >>> Coalescing that didn't really fit into anything and now we are
> >> > >>> starting to see other things related to DMA and PCIe bus credits.
> >> > >>> I'm
> >> > >>> wondering if we shouldn't start looking at a tool/interface to
> >> > >>> configure all the PCIe related features such as interrupts, error
> >> > >>> reporting, DMA configuration, power management, etc. Maybe we could
> >> > >>> even look at sharing it across subsystems and include things like
> >> > >>> storage, graphics, and other subsystems in the conversation.
> >> > >>
> >> > >>Agreed, for actual PCIe configuration (i.e. not ECN marking) we do
> >> > >> need
> >> > >>to build up an API. Sharing it across subsystems would be very cool!
> >>
> >> I read the thread (starting at [1], for anybody else coming in late)
> >> and I see this has something to do with "configuring outbound PCIe
> >> buffers", but I haven't seen the connection to PCIe protocol or
> >> features, i.e., I can't connect this to anything in the PCIe spec.
> >>
> >> Can somebody help me understand how the PCI core is relevant? If
> >> there's some connection with a feature defined by PCIe, or if it
> >> affects the PCIe transaction protocol somehow, I'm definitely
> >> interested in this. But if this only affects the data transferred
> >> over PCIe, i.e., the data payloads of PCIe TLP packets, then I'm not
> >> sure why the PCI core should care.
> >>
> >
> >
> > As you wrote, this is not a PCIe feature or affects the PCIe transaction
> > protocol.
> >
> > Actually, due to hardware limitation in current device, we have enabled a
> > workaround in hardware.
> >
> > This mode is proprietary and not relevant to other PCIe devices, thus is set
> > using driver-specific parameter in devlink
>
> Essentially what this feature is doing is communicating the need for
> PCIe back-pressure to the network fabric. So as the buffers on the
> device start to fill because the device isn't able to get back PCIe
> credits fast enough it will then start to send congestion
> notifications to the network stack itself if I understand this
> correctly.
This sounds like a hook that allows the device to tell its driver
about PCIe flow control credits, and the driver can pass that on to
the network stack. IIUC, that would be a device-specific feature
outside the scope of the PCI core.
> For now there are no major conflicts, but when we start getting into
> stuff like PCIe DMA coalescing, and on a more general basis just PCIe
> active state power management that is going to start making things
> more complicated going forward.
We do support ASPM already in the PCI core, and we do have the
pci_disable_link_state() interface, which is currently the only way
drivers can influence it. There are several drivers that do their own
ASPM configuration, but this is not safe because it's not coordinated
with what the PCI core does. If/when drivers need more control, we
should enhance the PCI core interfaces.
I don't know what PCIe DMA coalescing means, so I can't comment on
that.
> I assume the devices we are talking about supporting this new feature
> on either don't deal with ASPM or assume a quick turnaround to get out
> of the lower power states? Otherwise that would definitely cause some
> back-pressure buildups that would hurt performance.
Devices can communicate the ASPM exit latency they can tolerate via
the Device Capabilities register (PCIe r4.0, sec 76.5.3.3). Linux
should be configuring ASPM to respect those device requirements.
Bjorn
^ permalink raw reply
* Re: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect
From: Andrew Lunn @ 2018-07-30 14:06 UTC (permalink / raw)
To: Alexander Onnasch; +Cc: Florian Fainelli, netdev, linux-kernel
In-Reply-To: <1532959287-360-1-git-send-email-alexander.onnasch@landisgyr.com>
On Mon, Jul 30, 2018 at 04:01:27PM +0200, Alexander Onnasch wrote:
> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf describes the problem and possible workarounds in
> detail, see below.
> The patch implements workaround 1, which permanently fixes the issue.
> Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v5 1/4] net/sched: user-space can't set unknown tcfa_action values
From: Jamal Hadi Salim @ 2018-07-30 14:03 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Cong Wang, Jiri Pirko, Daniel Borkmann, Marcelo Ricardo Leitner,
Eyal Birger, David S. Miller
In-Reply-To: <951ad078ad4d5ee108abcd85189460aed7948634.1532934532.git.pabeni@redhat.com>
On 30/07/18 08:30 AM, Paolo Abeni wrote:
> }
>
> + if (!tcf_action_valid(a->tcfa_action)) {
> + NL_SET_ERR_MSG(extack, "invalid action value, using TC_ACT_UNSPEC instead");
> + a->tcfa_action = TC_ACT_UNSPEC;
> + }
> +
> return a;
>
I think it would make a lot more sense to just reject the entry than
changing it underneath the user to a default value. Least element of
suprise.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 00/10] mscc: ocelot: add support for SerDes muxing configuration
From: Andrew Lunn @ 2018-07-30 13:24 UTC (permalink / raw)
To: Quentin Schulz
Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, kishon, f.fainelli, linux-mips, devicetree,
linux-kernel, netdev, allan.nielsen, thomas.petazzoni
In-Reply-To: <cover.aa759035f6eefdd0bb2a5ae335dab5bd5399bd46.1532954208.git-series.quentin.schulz@bootlin.com>
> The SerDes configuration is in the middle of an address space (HSIO) that
> is used to configure some parts in the MAC controller driver, that is why
> we need to use a syscon so that we can write to the same address space from
> different drivers safely using regmap.
Hi Quentin
I assume breaking backwards compatibility is not an issue here, since
there currently is only one board using the DT binding. But it would
be good to give a warning in the cover notes. git bisect will also
break for this one particular board. And since these changes are going
through different trees, it could be quite a big break.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next v2 09/17] ethtool: implement GET_DRVINFO message
From: Jiri Pirko @ 2018-07-30 13:21 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, linux-kernel, David Miller, Florian Fainelli,
Roopa Prabhu, Jakub Kicinski, John W. Linville
In-Reply-To: <4dcd60f25efe368ada4e0c035dc1d7612ab59132.1532953989.git.mkubecek@suse.cz>
Mon, Jul 30, 2018 at 02:53:27PM CEST, mkubecek@suse.cz wrote:
[...]
>+/* GET_DRVINFO / SET_DRVINFO */
>+
>+enum {
>+ ETHA_DRVINFO_UNSPEC,
>+ ETHA_DRVINFO_DEV, /* nest - ETHA_DEV_* */
>+ ETHA_DRVINFO_DRIVER, /* string */
>+ ETHA_DRVINFO_VERSION, /* string */
>+ ETHA_DRVINFO_FWVERSION, /* string */
>+ ETHA_DRVINFO_BUSINFO, /* string */
>+ ETHA_DRVINFO_EROM_VER, /* string */
>+ ETHA_DRVINFO_N_PRIV_FLAGS, /* u32 */
>+ ETHA_DRVINFO_N_STATS, /* u32 */
>+ ETHA_DRVINFO_TESTINFO_LEN, /* u32 */
>+ ETHA_DRVINFO_EEDUMP_LEN, /* u32 */
>+ ETHA_DRVINFO_REGDUMP_LEN, /* u32 */
This is a nice example of why 1:1 ioctl->netlink conversion would be
a big mistake.
I understand that for ioclt, getting lengths of various things is
important. Userspace can prepare buffer for next ioctl which would
actually do dump transfer. However in netlink, this is totally pointless
as the dump goes into userspace in multiple netlink messages.
We need to figure out the netlink uapi from scratch.
[...]
^ permalink raw reply
* [PATCH bpf-next] lwt_bpf: remove unnecessary rcu_read_lock in run_lwt_bpf
From: Taehee Yoo @ 2018-07-30 13:22 UTC (permalink / raw)
To: daniel, ast; +Cc: netdev, ap420073
run_lwt_bpf is called by bpf_{input/output/xmit}.
These functions are already protected by rcu_read_lock.
because lwtunnel_{input/output/xmit} holds rcu_read_lock
and then calls bpf_{input/output/xmit}.
So that rcu_read_lock in the run_lwt_bpf is unnecessary.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
net/core/lwt_bpf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index e7e626f..a49c7ba 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -50,10 +50,8 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
* mixing with BH RCU lock doesn't work.
*/
preempt_disable();
- rcu_read_lock();
bpf_compute_data_pointers(skb);
ret = bpf_prog_run_save_cb(lwt->prog, skb);
- rcu_read_unlock();
switch (ret) {
case BPF_OK:
--
2.9.3
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 09/17] ethtool: implement GET_DRVINFO message
From: Michal Kubecek @ 2018-07-30 14:46 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-kernel, Jiri Pirko, David Miller, Florian Fainelli,
Roopa Prabhu, Jakub Kicinski, John W. Linville
In-Reply-To: <20180730142825.GL13198@lunn.ch>
On Mon, Jul 30, 2018 at 04:28:25PM +0200, Andrew Lunn wrote:
> On Mon, Jul 30, 2018 at 02:53:27PM +0200, Michal Kubecek wrote:
>
> > + ETHA_DRVINFO_FWVERSION (string) firmware version
> > + ETHA_DRVINFO_BUSINFO (string) device bus address
> > + ETHA_DRVINFO_EROM_VER (string) expansion ROM version
> > + ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags
> > + ETHA_DRVINFO_N_STATS (u32) number of device stats
>
> I know there is at least one driver that has somewhat dynamic number
> of statistics. It would be better to have the strings and the values
> in the same message, so there is no need to first get the number of
> strings, allocate the memory, get the strings, then get the values.
>
> > + ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results
>
> In theory, this also does not need to be fixed.
This is interesting. It would mean current (ioctl) ethtool approach with
string set may not work correctly either. On the other hand, this should
not be a problem for netlink interface. Statistics are unlikely to
appear in notifications and daemons collecting them periodically will
have to learn with it. Adding test name to notification "test started"
or "test finished" seems quite natural.
> > + ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size
> > + ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size
>
> I would suggest removing all these _LEN properties, and let netlink
> return what it needs to return.
Agreed.
Michal Kubecek
^ 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