* Re: Resource management for ndo_xdp_xmit (Was: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames)
From: Toke Høiland-Jørgensen @ 2019-02-09 16:56 UTC (permalink / raw)
To: Jakub Kicinski, Saeed Mahameed
Cc: brouer@redhat.com, hawk@kernel.org,
virtualization@lists.linux-foundation.org, borkmann@iogearbox.net,
Tariq Toukan, john.fastabend@gmail.com, mst@redhat.com,
dsahern@gmail.com, netdev@vger.kernel.org, jasowang@redhat.com,
davem@davemloft.net, makita.toshiaki@lab.ntt.co.jp
In-Reply-To: <20190208180516.287f6ddc@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Sat, 9 Feb 2019 00:18:31 +0000, Saeed Mahameed wrote:
>> On Fri, 2019-02-08 at 15:17 -0800, Saeed Mahameed wrote:
>> > On Thu, 2019-02-07 at 19:08 +0000, Saeed Mahameed wrote:
>> > >
>> > > So
>> > > 1) on dev_map_update_elem() we will call
>> > > dev->dev->ndo_bpf() to notify the device on the intention to
>> > > start/stop
>> > > redirect, and wait for it to create/destroy the HW resources
>> > > before/after actually updating the map
>> > >
>> >
>> > silly me, dev_map_update_elem must be atomic, we can't hook driver
>> > resource allocation to it, it must come as a separate request
>> > (syscall)
>> > from user space to request to create XDP redirect resources.
>> >
>>
>> Well, it is possible to render dev_map_update_elem non-atomic and fail
>> BPF programs who try to update it in the verifier
>> check_map_func_compatibility.
>>
>> if you know of any case where devmap needs to be updated from the BPF
>> program please let me know.
>
> Did we find a solution to non-map redirect?
See my other reply to Saeed :)
-Toke
^ permalink raw reply
* Re: [PATCH v1 net-next 3/4] net: dsa: microchip: use readx_poll_time for polling
From: Andrew Lunn @ 2019-02-09 17:01 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-4-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 07, 2019 at 08:07:08PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Replace register polling functions using timeout with readx_poll_time call.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> drivers/net/dsa/microchip/ksz9477.c | 91 +++++++++++--------------------------
> 1 file changed, 27 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 4502e13..8391b9e 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -114,28 +114,11 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
> data; \
> })
>
> -static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
> - int timeout)
> -{
> - u8 data;
> -
> - do {
> - ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> - if (!(data & waiton))
> - break;
> - usleep_range(1, 10);
> - } while (timeout-- > 0);
> -
> - if (timeout <= 0)
> - return -ETIMEDOUT;
> -
> - return 0;
Hi Tristram
I think it would be better to keep ksz9477_wait_vlan_ctrl_ready(),
ksz9477_wait_alu_ready() etc, and change there implementation to use
readx_poll_timeout(). The function names act as better documentation
for what we are waiting for than the parameters being passed to
readx_poll_timeout().
Andrew
^ permalink raw reply
* Re: [PATCH v1 net-next 4/4] net: dsa: microchip: remove unnecessary include headers
From: Andrew Lunn @ 2019-02-09 17:01 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-5-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 07, 2019 at 08:07:09PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Remove unnecessary header include lines.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Andrew Lunn @ 2019-02-09 17:22 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-3-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 07, 2019 at 08:07:07PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add MIB counter reading support.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> drivers/net/dsa/microchip/ksz9477.c | 139 +++++++++++++++++++++++----------
> drivers/net/dsa/microchip/ksz_common.c | 96 +++++++++++++++++++++++
> drivers/net/dsa/microchip/ksz_common.h | 2 +
> drivers/net/dsa/microchip/ksz_priv.h | 7 +-
> 4 files changed, 198 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 0fdb22d..4502e13 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -10,6 +10,7 @@
> #include <linux/gpio.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/iopoll.h>
> #include <linux/platform_data/microchip-ksz.h>
> #include <linux/phy.h>
> #include <linux/etherdevice.h>
> @@ -18,8 +19,8 @@
> #include <net/switchdev.h>
>
> #include "ksz_priv.h"
> -#include "ksz_common.h"
> #include "ksz9477_reg.h"
> +#include "ksz_common.h"
>
> static const struct {
> int index;
> @@ -92,6 +93,27 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
> ksz_write32(dev, addr, data);
> }
>
> +#define read8_op(addr) \
> +({ \
> + u8 data; \
> + ksz_read8(dev, addr, &data); \
> + data; \
> +})
> +
> +#define read32_op(addr) \
> +({ \
> + u32 data; \
> + ksz_read32(dev, addr, &data); \
> + data; \
> +})
These two are not used. Please remove them.
> +
> +#define pread32_op(addr) \
> +({ \
> + u32 data; \
> + ksz_pread32(dev, port, addr, &data); \
> + data; \
> +})
It works, but it is not nice, and it makes assumptions about how
readx_poll_timeout is implemented.
> + ret = readx_poll_timeout(pread32_op, REG_PORT_MIB_CTRL_STAT__4, data,
> + !(data & MIB_COUNTER_READ), 10, 1000);
The macro is only used one, and addr is fixed,
REG_PORT_MIB_CTRL_STAT__4. So you can at least replace addr with port,
and rename the macro pread32_stat(port).
> + /* failed to read MIB. get out of loop */
> + if (ret < 0) {
> + dev_dbg(dev->dev, "Failed to get MIB\n");
> + return;
> + }
> +
> + /* count resets upon read */
> + ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
> + *cnt += data;
> +}
> +
> +static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
> + u64 *dropped, u64 *cnt)
> +{
> + addr = ksz9477_mib_names[addr].index;
> + ksz9477_r_mib_cnt(dev, port, addr, cnt);
> +}
> +
> +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze)
> +{
> + struct ksz_port *p = &dev->ports[port];
> + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
Reverse Christmas tree.
> +
> + /* enable/disable the port for flush/freeze function */
> + mutex_lock(&p->mib.cnt_mutex);
> + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val);
> +
> + /* used by MIB counter reading code to know freeze is enabled */
> + p->freeze = freeze;
> + mutex_unlock(&p->mib.cnt_mutex);
> +}
> +static void ksz_mib_read_work(struct work_struct *work)
> +{
> + struct ksz_device *dev =
> + container_of(work, struct ksz_device, mib_read);
> + struct ksz_port *p;
> + struct ksz_port_mib *mib;
> + int i;
> +
> + for (i = 0; i < dev->mib_port_cnt; i++) {
> + p = &dev->ports[i];
> + if (!p->on)
> + continue;
> + mib = &p->mib;
> + mutex_lock(&mib->cnt_mutex);
> +
> + /* read only dropped counters when link is not up */
> + if (p->link_just_down)
> + p->link_just_down = 0;
> + else if (!p->phydev.link)
> + mib->cnt_ptr = dev->reg_mib_cnt;
This link_just_down stuff is not clear at all. Why can the drop
counters not be read when the link is up?
> + port_r_cnt(dev, i);
> + mutex_unlock(&mib->cnt_mutex);
> + }
> +}
^ permalink raw reply
* Re: [PATCH net-next] net/tls: Disable async decrytion for tls1.3
From: David Miller @ 2019-02-09 17:28 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20190209075110.19881-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Sat, 9 Feb 2019 07:53:28 +0000
> Function tls_sw_recvmsg() dequeues multiple records from stream parser
> and decrypts them. In case the decryption is done by async accelerator,
> the records may get submitted for decryption while the previous ones may
> not have been decryted yet. For tls1.3, the record type is known only
> after decryption. Therefore, for tls1.3, tls_sw_recvmsg() may submit
> records for decryption even if it gets 'handshake' records after 'data'
> records. These intermediate 'handshake' records may do a key updation.
> By the time new keys are given to ktls by userspace, it is possible that
> ktls has already submitted some records i(which are encrypted with new
> keys) for decryption using old keys. This would lead to decrypt failure.
> Therefore, async decryption of records should be disabled for tls1.3.
>
> Fixes: 130b392c6cd6b ("net: tls: Add tls 1.3 support")
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] net: phy: remove unneeded masking of PHY register read results
From: David Miller @ 2019-02-09 17:29 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <a644082b-3268-5070-ddaa-ff24ce9840c2@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 9 Feb 2019 09:46:53 +0100
> PHY registers are only 16 bits wide, therefore, if the read was
> successful, there's no need to mask out the higher 16 bits.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Applied, thanks Heiner.
^ permalink raw reply
* Re: [PATCH net-next 01/16] Documentation: networking: switchdev: Update port parent ID section
From: Jiri Pirko @ 2019-02-09 17:22 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-2-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:33AM CET, f.fainelli@gmail.com wrote:
>Update the section about switchdev drivers having to implement a
>switchdev_port_attr_get() function to return
>SWITCHDEV_ATTR_ID_PORT_PARENT_ID since that is no longer valid after
>commit bccb30254a4a ("net: Get rid of
>SWITCHDEV_ATTR_ID_PORT_PARENT_ID").
>
>Fixes: bccb30254a4a ("net: Get rid of SWITCHDEV_ATTR_ID_PORT_PARENT_ID")
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> Documentation/networking/switchdev.txt | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
>index f3244d87512a..2842f63ad47b 100644
>--- a/Documentation/networking/switchdev.txt
>+++ b/Documentation/networking/switchdev.txt
>@@ -92,11 +92,11 @@ device.
> Switch ID
> ^^^^^^^^^
>
>-The switchdev driver must implement the switchdev op switchdev_port_attr_get
>-for SWITCHDEV_ATTR_ID_PORT_PARENT_ID for each port netdev, returning the same
>-physical ID for each port of a switch. The ID must be unique between switches
>-on the same system. The ID does not need to be unique between switches on
>-different systems.
>+The switchdev driver must implement the net_device operation
>+ndo_get_port_parent_id for each port netdev, returning the same physical ID
Double space. Otherwise, this looks fine.
Acked-by: Jiri Pirko <jiri@mellanox.com>
>+for each port of a switch. The ID must be unique between switches on the same
>+system. The ID does not need to be unique between switches on different
>+systems.
>
> The switch ID is used to locate ports on a switch and to know if aggregated
> ports belong to the same switch.
>--
>2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next] net: phy: probe the PHY before determining the supported features
From: David Miller @ 2019-02-09 17:32 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <7163437e-3c36-0a6b-d328-1f54141c7a13@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 9 Feb 2019 14:46:30 +0100
> From: Andrew Lunn <andrew@lunn.ch>
> We will soon support asking the PHY at runtime to determine what
> features it supports, rather than forcing it to be compile time.
> But we should probe the PHY first. So probe the phy driver earlier.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: phy: Add support for asking the PHY its abilities
From: David Miller @ 2019-02-09 17:33 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <19c1e5c4-2f86-a3ff-e971-a0098c605b3f@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 9 Feb 2019 15:24:47 +0100
> From: Andrew Lunn <andrew@lunn.ch>
> Add support for runtime determination of what the PHY supports, by
> adding a new function to the phy driver. The get_features call should
> set the phydev->supported member with the features the PHY supports.
> It is only called if phydrv->features is NULL.
>
> This requires minor changes to pause. The PHY driver should not set
> pause abilities, except for when it has odd cause capabilities, e.g.
> pause cannot be disabled. With this change, phydev->supported already
> contains the drivers abilities, including pause. So rather than
> considering phydrv->features, look at the phydev->supported, and
> enable pause if neither of the pause bits are already set.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> [hkallweit1@gmail.com: fixed small checkpatch complaint in one comment]
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: marvell: mvpp2: clear flow control modes in 10G mode
From: David Miller @ 2019-02-09 17:34 UTC (permalink / raw)
To: rmk+kernel
Cc: antoine.tenart, maxime.chevallier, baruch, sven.auhagen, netdev
In-Reply-To: <E1gsV9X-00047r-QY@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Sat, 09 Feb 2019 16:06:51 +0000
> When mvpp2 configures the flow control modes in mvpp2_xlg_config() for
> 10G mode, it only ever set the flow control enable bits. There is no
> mechanism to clear these bits, which means that userspace is unable to
> use standard APIs to disable flow control (the only way is to poke the
> register directly.)
>
> Fix the missing bit clearance to allow flow control to be disabled.
> This means that, by default, as there is no negotiation in 10G modes
> with mvpp2, flow control is now disabled rather than being rx-only.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied.
^ permalink raw reply
* Re: [PATCH] net: mvpp2: clear flow control modes in 10G mode
From: David Miller @ 2019-02-09 17:34 UTC (permalink / raw)
To: linux; +Cc: antoine.tenart, maxime.chevallier, baruch, sven.auhagen, netdev
In-Reply-To: <20190209160708.e7o5wlcj2gt3f7cy@shell.armlinux.org.uk>
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Sat, 9 Feb 2019 16:07:08 +0000
> Please ignore this one - subject line is not correct. Thanks.
Aha, ok I'll apply the correct one.
^ permalink raw reply
* Re: [PATCH] net: hso: do not unregister if not registered
From: David Miller @ 2019-02-09 17:35 UTC (permalink / raw)
To: tuba; +Cc: netdev
In-Reply-To: <89a423a522834affadda4aaf7ab4d333@exmbxprd18.ad.ufl.edu>
From: "Yavuz, Tuba" <tuba@ece.ufl.edu>
Date: Sat, 9 Feb 2019 16:24:20 +0000
>
>
> On an error path inside the hso_create_net_device function of the hso
> driver, hso_free_net_device gets called. This causes potentially a
> negative reference count in the net device if register_netdev has not
> been called yet as hso_free_net_device calls unregister_netdev
> regardless. I think the driver should distinguish these cases and call
> unregister_netdev only if register_netdev has been called.
>
> Signed-off-by: Tuba Yavuz <tuba@ece.ufl.edu>
Your patch is still corrupted.
Email this patch to yourself and try to apply it to a source tree.
You will see that it won't work.
Please do not post this patch again until you have fixed this problem
and have also successfully applied a patch you have test emailed to
yourself.
Thanks.
^ permalink raw reply
* Re: TC stats / hw offload question
From: Jamal Hadi Salim @ 2019-02-09 17:39 UTC (permalink / raw)
To: Edward Cree, netdev
Cc: Jiri Pirko, Cong Wang, Or Gerlitz, Andy Gospodarek, PJ Waskiewicz,
Anjali Singhai Jain, Jakub Kicinski
In-Reply-To: <70d77198-42fd-838a-d352-2647e7cad4d6@solarflare.com>
On 2019-02-08 5:26 a.m., Edward Cree wrote:
> On 06/02/19 02:20, Jamal Hadi Salim wrote:
>> tc match using flower blah \
>> action vlan push tag ... \
>> action redirect to egress of eth0
>>
>> And you submited a packet of size x bytes,
>> then the "match" would record x bytes.
>
> Sorry, where would it record that?
Sorry - that was hypothetical on my part. We dont
record the bytes in the classifier. We do (on some
classifiers) record hit counts, so we can see how many
times a lookup on a specific filter happened and how
many times that lookup resulted in a match.
See u32 (or a few others Cong has been updating
recently).
> I can't find any stats counters on
> the "match" either in the software path or the offload API.
Hasnt been necessary thus far.
Is your end goal to match and count?
Most hardware has stats/counters as a separate table.
Assuming yours does as well, then if all you want was
to just match and accept, then you would add a filter
with an accept/ok. i.e something along the lines of:
tc match using flower blah \
action ok index 12345
The "action index" field can be used to map to a counter
table index in hardware. Note if you dont explicitly
specify the index, the kernel (and in this case h/w)
issues one for you.
>> the "vlan action" would record x bytes.
>> the "redirect" would record size x+vlaninfo bytes
>> the egress of eth0 would recorr x+vlaninfo bytes
> Am I right in thinking that offloaded counters don't do that? As far
> as I can tell, the drivers with flower offload all use
> tcf_exts_stats_update() which takes a single 'bytes' count and adds
> it to all the actions. (Presumably this is pre-edit length.)
On the pre/post edit, perhaps some of the hardware
owners could chime in +Ccing a few.
To the tcf_exts_stats_update():
Note, most people interested in h/w offload will choose to skip
sw (explicitly defined when you add a rule).
The update synchronizes the hardware stats/counters
to the kernel - so when you dump the policies in such a setup
you are seeing what is reflected in hardware.
cheers,
jamal
PS: I have to say just keeping one counter is at times
insufficient. Example, the policer records how many bytes
were seen, not how many bytes were allowed through.
For tunnels, it is easy to compute post-edit size because
you can easily compute later the added/removed bytes.
^ permalink raw reply
* Re: [PATCH net-next 02/16] mlxsw: spectrum: Check bridge flags during prepare phase
From: Jiri Pirko @ 2019-02-09 17:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-3-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:34AM CET, f.fainelli@gmail.com wrote:
>In preparation for getting rid of switchdev_port_attr_get(), have mlxsw
>check for the bridge flags being set through switchdev_port_attr_set()
>with the SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS attribute identifier.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 03/16] staging: fsl-dpaa2: ethsw: Check bridge port flags during set
From: Jiri Pirko @ 2019-02-09 17:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-4-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:35AM CET, f.fainelli@gmail.com wrote:
>In preparation for removing SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
>have ethsw check that the bridge port flags that are being set are
>supported.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 04/16] net: dsa: Add setter for SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
From: Jiri Pirko @ 2019-02-09 17:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-5-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:36AM CET, f.fainelli@gmail.com wrote:
>In preparation for removing SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
>add support for a function that processes the
>SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS attribute and returns not supported
>for any flag set, since DSA does not currently support toggling those
>bridge port attributes (yet).
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 05/16] rocker: Check bridge flags during prepare phase
From: Jiri Pirko @ 2019-02-09 17:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-6-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:37AM CET, f.fainelli@gmail.com wrote:
>In preparation for getting rid of switchdev_port_attr_get(), have rocker
>check for the bridge flags being set through switchdev_port_attr_set()
>with the SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS attribute identifier.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 06/16] net: bridge: Stop calling switchdev_port_attr_get()
From: Jiri Pirko @ 2019-02-09 17:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-7-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:38AM CET, f.fainelli@gmail.com wrote:
>Now that all switchdev drivers have been converted to checking the
>bridge port flags during the prepare phase of the
>switchdev_port_attr_set(), we can move straight to trying to set the
>desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 07/16] net: Remove SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
From: Jiri Pirko @ 2019-02-09 17:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-8-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:39AM CET, f.fainelli@gmail.com wrote:
>Now that we have converted the bridge code and the drivers to check for
>bridge port(s) flags at the time we try to set them, there is no need
>for a get() -> set() sequence anymore and
>SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT therefore becomes unused.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 08/16] net: Get rid of switchdev_port_attr_get()
From: Jiri Pirko @ 2019-02-09 18:15 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-9-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:40AM CET, f.fainelli@gmail.com wrote:
>With the bridge no longer calling switchdev_port_attr_get() to obtain
>the supported bridge port flags from a driver but instead trying to set
>the bridge port flags directly and relying on driver to reject
>unsupported configurations, we can effectively get rid of
>switchdev_port_attr_get() entirely since this was the only place where
>it was called.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 09/16] switchdev: Add SWITCHDEV_PORT_ATTR_SET
From: Jiri Pirko @ 2019-02-09 18:18 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-10-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:41AM CET, f.fainelli@gmail.com wrote:
>In preparation for allowing switchdev enabled drivers to veto specific
>attribute settings from within the context of the caller, introduce a
>new switchdev notifier type for port attributes.
>
>Suggested-by: Ido Schimmel <idosch@mellanox.com>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> include/net/switchdev.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index 96cd3e795f7f..4c5f7e5430cf 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -141,6 +141,8 @@ enum switchdev_notifier_type {
> SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE,
> SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE,
> SWITCHDEV_VXLAN_FDB_OFFLOADED,
>+
>+ SWITCHDEV_PORT_ATTR_SET, /* Blocking. */
This is not UAPI. You can put it next to SWITCHDEV_PORT_OBJ_ADD and
SWITCHDEV_PORT_OBJ_DEL. Up to you.
With or without that:
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 10/16] rocker: Handle SWITCHDEV_PORT_ATTR_SET
From: Jiri Pirko @ 2019-02-09 18:21 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209003248.31088-11-f.fainelli@gmail.com>
Sat, Feb 09, 2019 at 01:32:42AM CET, f.fainelli@gmail.com wrote:
>Following patches will change the way we communicate getting or setting
Just "setting", no "getting".
>a port's attribute and use a blocking notifier to perform those tasks.
>
>Prepare rocker to support receiving notifier events targeting
>SWITCHDEV_PORT_ATTR_SET and simply translate that into the existing
>rocker_port_attr_set call.
>
>Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> drivers/net/ethernet/rocker/rocker_main.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
>diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
>index ff3f14504f4f..f10e4888ecff 100644
>--- a/drivers/net/ethernet/rocker/rocker_main.c
>+++ b/drivers/net/ethernet/rocker/rocker_main.c
>@@ -2811,6 +2811,24 @@ rocker_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
> return notifier_from_errno(err);
> }
>
>+static int
>+rocker_switchdev_port_attr_event(unsigned long event, struct net_device *netdev,
>+ struct switchdev_notifier_port_attr_info
>+ *port_attr_info)
>+{
>+ int err = -EOPNOTSUPP;
>+
>+ switch (event) {
>+ case SWITCHDEV_PORT_ATTR_SET:
Do you expect some other event to be handled in
rocker_switchdev_port_attr_event()? Because you have SWITCHDEV_PORT_ATTR_SET
selected in case here and in rocker_switchdev_blocking_event.
Perhaps you can rename rocker_switchdev_port_attr_event() to
rocker_switchdev_port_attr_set_event() and avoid this switchcase.
>+ err = rocker_port_attr_set(netdev, port_attr_info->attr,
>+ port_attr_info->trans);
>+ break;
>+ }
>+
>+ port_attr_info->handled = true;
>+ return notifier_from_errno(err);
>+}
>+
> static int rocker_switchdev_blocking_event(struct notifier_block *unused,
> unsigned long event, void *ptr)
> {
>@@ -2823,6 +2841,8 @@ static int rocker_switchdev_blocking_event(struct notifier_block *unused,
> case SWITCHDEV_PORT_OBJ_ADD:
> case SWITCHDEV_PORT_OBJ_DEL:
> return rocker_switchdev_port_obj_event(event, dev, ptr);
>+ case SWITCHDEV_PORT_ATTR_SET:
>+ return rocker_switchdev_port_attr_event(event, dev, ptr);
> }
>
> return NOTIFY_DONE;
>--
>2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next 10/16] rocker: Handle SWITCHDEV_PORT_ATTR_SET
From: Jiri Pirko @ 2019-02-09 18:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David S. Miller, Ido Schimmel, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
andrew, vivien.didelot
In-Reply-To: <20190209182147.GQ2353@nanopsycho>
Sat, Feb 09, 2019 at 07:21:47PM CET, jiri@resnulli.us wrote:
[...]
>>+static int
>>+rocker_switchdev_port_attr_event(unsigned long event, struct net_device *netdev,
>>+ struct switchdev_notifier_port_attr_info
>>+ *port_attr_info)
>>+{
>>+ int err = -EOPNOTSUPP;
>>+
>>+ switch (event) {
>>+ case SWITCHDEV_PORT_ATTR_SET:
>
>Do you expect some other event to be handled in
>rocker_switchdev_port_attr_event()? Because you have SWITCHDEV_PORT_ATTR_SET
>selected in case here and in rocker_switchdev_blocking_event.
>Perhaps you can rename rocker_switchdev_port_attr_event() to
>rocker_switchdev_port_attr_set_event() and avoid this switchcase.
Same comment applies on next 4 patches.
^ permalink raw reply
* Re: [PATCH net-next] net: phy: Add support for asking the PHY its abilities
From: Heiner Kallweit @ 2019-02-09 19:12 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20190209163554.GB30856@lunn.ch>
On 09.02.2019 17:35, Andrew Lunn wrote:
> On Sat, Feb 09, 2019 at 03:24:47PM +0100, Heiner Kallweit wrote:
>> From: Andrew Lunn <andrew@lunn.ch>
>> Add support for runtime determination of what the PHY supports, by
>> adding a new function to the phy driver. The get_features call should
>> set the phydev->supported member with the features the PHY supports.
>> It is only called if phydrv->features is NULL.
>>
>> This requires minor changes to pause. The PHY driver should not set
>> pause abilities, except for when it has odd cause capabilities, e.g.
>> pause cannot be disabled. With this change, phydev->supported already
>> contains the drivers abilities, including pause. So rather than
>> considering phydrv->features, look at the phydev->supported, and
>> enable pause if neither of the pause bits are already set.
>
> Hi Heiner
>
> Ah, cool, these are the patches i was asking for, when you asked
> about splitting Maxime's patches. There is one more in my tree which
> converts the marvell10g to using this. I think that should be posted
> as well. It makes it clear how this should be used, and it replaces
> one of the patches in Maxime's set.
>
I know, it's patch 15 in your series ;) And I'm aware that usually new
core functionality is acceptable only if it comes together with a user,
to avoid having billions of orphaned good ideas in the code.
I focused on the core here to not get lost in all the new stuff, and to
provide Maxime with some direction for adjusting his series.
>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>> [hkallweit1@gmail.com: fixed small checkpatch complaint in one comment]
>
> Thanks.
>
> Andrew
>
Heiner
^ permalink raw reply
* Re: [PATCH][next] can: at91_can: mark expected switch fall-throughs
From: Sergei Shtylyov @ 2019-02-09 19:17 UTC (permalink / raw)
To: Gustavo A. R. Silva, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches
Cc: linux-can, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <8402d6f6-dff1-eb99-3bd1-051b4f6f24f0@cogentembedded.com>
On 02/08/2019 09:55 PM, Sergei Shtylyov wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch
>> cases where we are expecting to fall through.
>>
>> Notice that, in this particular case, the /* fall through */
>> comments are placed at the bottom of the case statement, which
>> is what GCC is expecting to find.
>>
>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>
>> This patch is part of the ongoing efforts to enabling
>> -Wimplicit-fallthrough.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> drivers/net/can/at91_can.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>> index d98c69045b17..1718c20f9c99 100644
>> --- a/drivers/net/can/at91_can.c
>> +++ b/drivers/net/can/at91_can.c
>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
>> CAN_ERR_CRTL_TX_WARNING :
>> CAN_ERR_CRTL_RX_WARNING;
>> }
>> - case CAN_STATE_ERROR_WARNING: /* fallthrough */
>> + /* fall through */
>
> Why do we need this comment at all? Just remove it, that's all.
>
>> + case CAN_STATE_ERROR_WARNING:
>> /*
>> * from: ERROR_ACTIVE, ERROR_WARNING
>> * to : ERROR_PASSIVE, BUS_OFF
>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
>> netdev_dbg(dev, "Error Active\n");
>> cf->can_id |= CAN_ERR_PROT;
>> cf->data[2] = CAN_ERR_PROT_ACTIVE;
>> - case CAN_STATE_ERROR_WARNING: /* fallthrough */
>> + /* fall through */
>
> Again, we don;t need it here.
>
>> + case CAN_STATE_ERROR_WARNING:
>> reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
>> reg_ier = AT91_IRQ_ERRP;
>> break;
Ignore me, I misread the code...
MBR, Sergei
^ 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