Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ipv6: Fix the link time qualifier of 'ping_v6_proc_exit_net()'
From: David Miller @ 2019-09-12 10:21 UTC (permalink / raw)
  To: christophe.jaillet
  Cc: kuznet, yoshfuji, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190910112959.9222-1-christophe.jaillet@wanadoo.fr>

From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Tue, 10 Sep 2019 13:29:59 +0200

> The '.exit' functions from 'pernet_operations' structure should be marked
> as __net_exit, not __net_init.
> 
> Fixes: d862e5461423 ("net: ipv6: Implement /proc/net/icmp6.")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Untested, but using __net_exit looks consistent with other
> pernet_operations.exit use case.

Looks good, applied.

^ permalink raw reply

* [PATCH v2] net: stmmac: socfpga: re-use the `interface` parameter from platform data
From: Alexandru Ardelean @ 2019-09-12 13:28 UTC (permalink / raw)
  To: netdev, linux-stm32, linux-arm-kernel, linux-kernel
  Cc: peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
	davem, Alexandru Ardelean

The socfpga sub-driver defines an `interface` field in the `socfpga_dwmac`
struct and parses it on init.

The shared `stmmac_probe_config_dt()` function also parses this from the
device-tree and makes it available on the returned `plat_data` (which is
the same data available via `netdev_priv()`).

All that's needed now is to dig that information out, via some
`dev_get_drvdata()` && `netdev_priv()` calls and re-use it.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* initially, this patch was developed on a 4.14 kernel, and adapted (badly)
  to `net-next`, so it did not build ; the v2 has been fixed and adapted
  correctly

 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c   | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index c141fe783e87..5b6213207c43 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -46,7 +46,6 @@ struct socfpga_dwmac_ops {
 };
 
 struct socfpga_dwmac {
-	int	interface;
 	u32	reg_offset;
 	u32	reg_shift;
 	struct	device *dev;
@@ -110,8 +109,6 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 	struct resource res_tse_pcs;
 	struct resource res_sgmii_adapter;
 
-	dwmac->interface = of_get_phy_mode(np);
-
 	sys_mgr_base_addr =
 		altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
 	if (IS_ERR(sys_mgr_base_addr)) {
@@ -231,6 +228,14 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 	return ret;
 }
 
+static inline int socfpga_get_plat_phymode(struct socfpga_dwmac *dwmac)
+{
+	struct net_device *ndev = dev_get_drvdata(dwmac->dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	return priv->plat->interface;
+}
+
 static int socfpga_set_phy_mode_common(int phymode, u32 *val)
 {
 	switch (phymode) {
@@ -255,7 +260,7 @@ static int socfpga_set_phy_mode_common(int phymode, u32 *val)
 static int socfpga_gen5_set_phy_mode(struct socfpga_dwmac *dwmac)
 {
 	struct regmap *sys_mgr_base_addr = dwmac->sys_mgr_base_addr;
-	int phymode = dwmac->interface;
+	int phymode = socfpga_get_plat_phymode(dwmac);
 	u32 reg_offset = dwmac->reg_offset;
 	u32 reg_shift = dwmac->reg_shift;
 	u32 ctrl, val, module;
@@ -314,7 +319,7 @@ static int socfpga_gen5_set_phy_mode(struct socfpga_dwmac *dwmac)
 static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac)
 {
 	struct regmap *sys_mgr_base_addr = dwmac->sys_mgr_base_addr;
-	int phymode = dwmac->interface;
+	int phymode = socfpga_get_plat_phymode(dwmac);
 	u32 reg_offset = dwmac->reg_offset;
 	u32 reg_shift = dwmac->reg_shift;
 	u32 ctrl, val, module;
-- 
2.20.1


^ permalink raw reply related

* Re: [Patch net] sch_sfb: fix a crash in sfb_destroy()
From: Linus Torvalds @ 2019-09-12 10:31 UTC (permalink / raw)
  To: Cong Wang
  Cc: Eric Dumazet, Linux Kernel Network Developers, syzbot,
	Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <CAM_iQpVP6qVbWmV+kA8UGXG6r1LJftyV32UjUbqryGrX5Ud8Nw@mail.gmail.com>

On Thu, Sep 12, 2019 at 2:10 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 2:36 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > It seems a similar fix would be needed in net/sched/sch_dsmark.c ?
> >
>
> Yeah, or just add a NULL check in dsmark_destroy().

Well, this was why one of my suggestions was to just make
"qdisc_put()" be happy with a NULL pointer (or even an ERR_PTR()).

That would have fixed not just sfb, but also dsmark with a single patch.

We tend to have that kind of pattern in a lot of places, where we can
free unallocated structures (end ERR_PTR() pointers) withour errors,
so that

  destroy_fn(alloc_fn());

is defined to always work, even when alloc_fn() returns NULL or an
error. That, and allowing the "it was never allocated at all" case (as
long as it's initialized to NULL, of course) tends to make various
error cases simpler.

The obvious one is kfree(kmalloc()), of course, but we've done it in
other places too. So you find things like

  void i2c_unregister_device(struct i2c_client *client)
  {
        if (IS_ERR_OR_NULL(client))
                return;

in various subsystems and drivers. So one of my suggestions was to
just do that to qdisc_put().

It depends on what you want to do, of course. Do you want to make sure
each user is being very careful? Or do you want to make the interfaces
easy to use without _having_ to be careful? There are arguments both
ways, but we've tended to move more towards a "easy to use" model than
the "be careful" one.

               Linus

^ permalink raw reply

* Re: [PATCH net-next v2 0/3] add ksz9567 with I2C support to ksz9477 driver
From: David Miller @ 2019-09-12 10:39 UTC (permalink / raw)
  To: george.mccollister
  Cc: netdev, woojung.huh, andrew, f.fainelli, Tristram.Ha, marex,
	linux-kernel
In-Reply-To: <20190910131836.114058-1-george.mccollister@gmail.com>

From: George McCollister <george.mccollister@gmail.com>
Date: Tue, 10 Sep 2019 08:18:33 -0500

> Resurrect KSZ9477 I2C driver support patch originally sent to the list
> by Tristram Ha and resolve outstanding issues. It now works as similarly to
> the ksz9477 SPI driver as possible, using the same regmap macros.
> 
> Add support for ksz9567 to the ksz9477 driver (tested on a board with
> ksz9567 connected via I2C).
> 
> Remove NET_DSA_TAG_KSZ_COMMON since it's not needed.
> 
> Changes since v1:
> Put ksz9477_i2c.c includes in alphabetical order.
> Added Reviewed-Bys.

Series applied.

Please follow up with Andrew about the macros.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
From: Andrew Lunn @ 2019-09-12 10:43 UTC (permalink / raw)
  To: Robert Beckett
  Cc: Florian Fainelli, netdev, Vivien Didelot, David S. Miller,
	bob.beckett
In-Reply-To: <8d63d4dbd9d075b5c238fd8933673b95b2fa96e9.camel@collabora.com>

> > It actually has nothing to do with PHY to PHY connections. You can
> > use
> > pause frames with direct MAC to MAC connections. PHY auto-negotiation
> > is one way to indicate both ends support it, but there are also other
> > ways. e.g.
> > 
> > ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]
> > 
> > on the SoC you could do
> > 
> > ethtool --pause eth0 autoneg off rx on tx on
> > 
> > to force the SoC to send and process pause frames. Ideally i would
> > prefer a solution like this, since it is not a change of behaviour
> > for
> > everybody else.
> 
> Good point, well made.
> The reason for using autoneg in this series was due to having no netdev
> to run ethtool against for the CPU port.

Do you need one? It is the IMX which is the bottle neck. It is the one
which needs to send pause frames. You have a netdev for that. Have you
checked if the switch will react on pause frames without your
change. Play with the command i give above on the master interface. It
looks like the FEC driver fully supports synchronous pause
configuration.

> However, given that the phy on the marvell switch is capable of
> autoneg , is it not reasonable to setup the advertisement and let
> autoneg take care of it if using phy to phy connection?

Most designs don't use back to back PHYs for the CPU port. They save
the cost and connect MACs back to back using RGMII, or maybe SERDES.
If we are going for a method which can configure pause between the CPU
and the switch, it needs to be generic and work for both setups.

    Andrew

^ permalink raw reply

* Re: [PATCH] bpf: validate bpf_func when BPF_JIT is enabled
From: Toke Høiland-Jørgensen @ 2019-09-12 10:46 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: Björn Töpel, Yonghong Song, Alexei Starovoitov,
	Daniel Borkmann, Kees Cook, Martin Lau, Song Liu,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jesper Dangaard Brouer
In-Reply-To: <CABCJKufCwjXQ6a4oLjywDmxY2apUZ1yop-5+qty82bfwV-QTAA@mail.gmail.com>

Sami Tolvanen <samitolvanen@google.com> writes:

> On Wed, Sep 11, 2019 at 5:09 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Björn Töpel <bjorn.topel@intel.com> writes:
>> > I ran the "xdp_rxq_info" sample with and without Sami's patch:
>>
>> Thanks for doing this!
>
> Yes, thanks for testing this Björn!
>
>> Or (1/22998700 - 1/23923874) * 10**9 == 1.7 nanoseconds of overhead.
>>
>> I guess that is not *too* bad; but it's still chipping away at
>> performance; anything we could do to lower the overhead?
>
> The check is already rather minimal, but I could move this to a static
> inline function to help ensure the compiler doesn't generate an
> additional function call for this. I'm also fine with gating this
> behind a separate config option, but I'm not sure if that's worth it.
> Any thoughts?

I think it would be good if you do both. I'm a bit worried that XDP
performance will end up in a "death by a thousand paper cuts" situation,
so I'd rather push back on even relatively small overheads like this; so
being able to turn it off in the config would be good.

Can you share more details about what the "future CFI checking" is
likely to look like?

-Toke

^ permalink raw reply

* Re: [PATCH net] net: Fix null de-reference of device refcount
From: David Miller @ 2019-09-12 10:56 UTC (permalink / raw)
  To: subashab; +Cc: dlezcano, eric.dumazet, netdev, stranche
In-Reply-To: <1568145777-29480-1-git-send-email-subashab@codeaurora.org>

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Tue, 10 Sep 2019 14:02:57 -0600

> In event of failure during register_netdevice, free_netdev is
> invoked immediately. free_netdev assumes that all the netdevice
> refcounts have been dropped prior to it being called and as a
> result frees and clears out the refcount pointer.
> 
> However, this is not necessarily true as some of the operations
> in the NETDEV_UNREGISTER notifier handlers queue RCU callbacks for
> invocation after a grace period. The IPv4 callback in_dev_rcu_put
> tries to access the refcount after free_netdev is called which
> leads to a null de-reference-
 ...
> Fix this by waiting for the completion of the call_rcu() in
> case of register_netdevice errors.
> 
> Fixes: 93ee31f14f6f ("[NET]: Fix free_netdev on register_netdev failure.")
> Cc: Sean Tranchetti <stranche@codeaurora.org>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v2] net: qrtr: fix memort leak in qrtr_tun_write_iter
From: David Miller @ 2019-09-12 10:59 UTC (permalink / raw)
  To: navid.emamdoost; +Cc: emamd001, smccaman, kjlu, netdev, linux-kernel
In-Reply-To: <20190911150907.18251-1-navid.emamdoost@gmail.com>

From: Navid Emamdoost <navid.emamdoost@gmail.com>
Date: Wed, 11 Sep 2019 10:09:02 -0500

> In qrtr_tun_write_iter the allocated kbuf should be release in case of
> error or success return.
> 
> v2 Update: Thanks to David Miller for pointing out the release on success
> path as well.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>

Applied, thanks.

^ permalink raw reply

* [PATCH] ixgbe: Fix secpath usage for IPsec TX offload.
From: Steffen Klassert @ 2019-09-12 11:01 UTC (permalink / raw)
  To: Jeff Kirsher, intel-wired-lan; +Cc: Michael Marley, Shannon Nelson, netdev

The ixgbe driver currently does IPsec TX offloading
based on an existing secpath. However, the secpath
can also come from the RX side, in this case it is
misinterpreted for TX offload and the packets are
dropped with a "bad sa_idx" error. Fix this by using
the xfrm_offload() function to test for TX offload.

Fixes: 592594704761 ("ixgbe: process the Tx ipsec offload")
Reported-by: Michael Marley <michael@michaelmarley.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 9bcae44e9883..ae31bd57127c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -36,6 +36,7 @@
 #include <net/vxlan.h>
 #include <net/mpls.h>
 #include <net/xdp_sock.h>
+#include <net/xfrm.h>
 
 #include "ixgbe.h"
 #include "ixgbe_common.h"
@@ -8696,7 +8697,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 #endif /* IXGBE_FCOE */
 
 #ifdef CONFIG_IXGBE_IPSEC
-	if (secpath_exists(skb) &&
+	if (xfrm_offload(skb) &&
 	    !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx))
 		goto out_drop;
 #endif
-- 
2.17.1


^ permalink raw reply related

* Re: ANNOUNCE: rpld an another RPL implementation for Linux
From: Stefan Schmidt @ 2019-09-12 11:14 UTC (permalink / raw)
  To: Alexander Aring, open list:NETWORKING [GENERAL]
  Cc: Michael Richardson, Jamal Hadi Salim, Robert Kaiser,
	Martin Gergeleit, Kai Beckmann, koen, linux-wpan - ML, reubenhwk,
	BlueZ development, sebastian.meiling, Marcel Holtmann,
	Werner Almesberger, Jukka Rissanen
In-Reply-To: <CAB_54W7h9ca0UJAZtk=ApPX-2ZCvzu4774BTFTaB5mtkobWCtw@mail.gmail.com>

Hello Alex.

On 29.08.19 23:57, Alexander Aring wrote:
> Hi,
> 
> I had some free time, I wanted to know how RPL [0] works so I did a
> implementation. It's _very_ basic as it only gives you a "routable"
> (is that a word?) thing afterwards in a very constrained setup of RPL
> messages.
> 
> Took ~1 month to implement it and I reused some great code from radvd
> [1]. I released it under the same license (BSD?). Anyway, I know there
> exists a lot of memory leaks and the parameters are just crazy as not
> practical in a real environment BUT it works.
> 
> I changed a little bit the dependencies from radvd (because fancy new things):
> 
> - lua for config handling
> - libev for event loop handling
> - libmnl for netlink handling
> 
> The code is available at:
> 
> https://github.com/linux-wpan/rpld

I finally had a first look at it and played around a little bit.

How do you want to review patches for this? Pull requests on the github
repo or patches send on the linux-wpan list?

So far just some basic stuff I stumbled over when playing with it. Build
fixes (SCOPE_ID and different lua pkgconfig namings), leak fixes to
config.c as well as a travis setup to get building on CI as well as
submitting to Coverity scan service (the later two are already tested in
practice with some dev branches I pushed to the github repo, hope you
don't mind).

regards
Stefan Schmidt

^ permalink raw reply

* [patch iproute2-next v4 0/2] devlink: couple forgotten flash patches
From: Jiri Pirko @ 2019-09-12 11:29 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, jakub.kicinski, saeedm, mlxsw, f.fainelli

From: Jiri Pirko <jiri@mellanox.com>

I was under impression they are already merged, but apparently they are
not. I just rebased them on top of current iproute2 net-next tree.

Jiri Pirko (2):
  devlink: implement flash update status monitoring
  devlink: implement flash status monitoring

 devlink/devlink.c      | 258 ++++++++++++++++++++++++++++++++++++++++-
 devlink/mnlg.c         |   5 +
 devlink/mnlg.h         |   1 +
 man/man8/devlink-dev.8 |  11 ++
 4 files changed, 271 insertions(+), 4 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [patch iproute2-next v4 1/2] devlink: implement flash update status monitoring
From: Jiri Pirko @ 2019-09-12 11:29 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, jakub.kicinski, saeedm, mlxsw, f.fainelli
In-Reply-To: <20190912112938.2292-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Kernel sends notifications about flash update status, so implement these
messages for monitoring.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- rebased (traps)
---
 devlink/devlink.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 2f084c020765..31c319e3ef7a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -443,6 +443,10 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] = MNL_TYPE_U64,
 	[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS] = MNL_TYPE_U64,
 	[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = MNL_TYPE_U64,
+	[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT] = MNL_TYPE_STRING,
+	[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG] = MNL_TYPE_STRING,
+	[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE] = MNL_TYPE_U64,
+	[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL] = MNL_TYPE_U64,
 	[DEVLINK_ATTR_STATS] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_TRAP_NAME] = MNL_TYPE_STRING,
 	[DEVLINK_ATTR_TRAP_ACTION] = MNL_TYPE_U8,
@@ -3878,6 +3882,9 @@ static const char *cmd_name(uint8_t cmd)
 	case DEVLINK_CMD_REGION_SET: return "set";
 	case DEVLINK_CMD_REGION_NEW: return "new";
 	case DEVLINK_CMD_REGION_DEL: return "del";
+	case DEVLINK_CMD_FLASH_UPDATE: return "begin";
+	case DEVLINK_CMD_FLASH_UPDATE_END: return "end";
+	case DEVLINK_CMD_FLASH_UPDATE_STATUS: return "status";
 	case DEVLINK_CMD_TRAP_GET: return "get";
 	case DEVLINK_CMD_TRAP_SET: return "set";
 	case DEVLINK_CMD_TRAP_NEW: return "new";
@@ -3914,6 +3921,10 @@ static const char *cmd_obj(uint8_t cmd)
 	case DEVLINK_CMD_REGION_NEW:
 	case DEVLINK_CMD_REGION_DEL:
 		return "region";
+	case DEVLINK_CMD_FLASH_UPDATE:
+	case DEVLINK_CMD_FLASH_UPDATE_END:
+	case DEVLINK_CMD_FLASH_UPDATE_STATUS:
+		return "flash";
 	case DEVLINK_CMD_TRAP_GET:
 	case DEVLINK_CMD_TRAP_SET:
 	case DEVLINK_CMD_TRAP_NEW:
@@ -3948,6 +3959,29 @@ static bool cmd_filter_check(struct dl *dl, uint8_t cmd)
 	return false;
 }
 
+static void pr_out_flash_update(struct dl *dl, struct nlattr **tb)
+{
+	__pr_out_handle_start(dl, tb, true, false);
+
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG])
+		pr_out_str(dl, "msg",
+			   mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG]));
+
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT])
+		pr_out_str(dl, "component",
+			   mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT]));
+
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE])
+		pr_out_u64(dl, "done",
+			   mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE]));
+
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL])
+		pr_out_u64(dl, "total",
+			   mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL]));
+
+	pr_out_handle_end(dl);
+}
+
 static void pr_out_region(struct dl *dl, struct nlattr **tb);
 static void pr_out_trap(struct dl *dl, struct nlattr **tb, bool array);
 static void pr_out_trap_group(struct dl *dl, struct nlattr **tb, bool array);
@@ -4006,6 +4040,15 @@ static int cmd_mon_show_cb(const struct nlmsghdr *nlh, void *data)
 		pr_out_mon_header(genl->cmd);
 		pr_out_region(dl, tb);
 		break;
+	case DEVLINK_CMD_FLASH_UPDATE: /* fall through */
+	case DEVLINK_CMD_FLASH_UPDATE_END: /* fall through */
+	case DEVLINK_CMD_FLASH_UPDATE_STATUS:
+		mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+		if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
+			return MNL_CB_ERROR;
+		pr_out_mon_header(genl->cmd);
+		pr_out_flash_update(dl, tb);
+		break;
 	case DEVLINK_CMD_TRAP_GET: /* fall through */
 	case DEVLINK_CMD_TRAP_SET: /* fall through */
 	case DEVLINK_CMD_TRAP_NEW: /* fall through */
-- 
2.20.1


^ permalink raw reply related

* [patch iproute2-next v4 2/2] devlink: implement flash status monitoring
From: Jiri Pirko @ 2019-09-12 11:29 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern, jakub.kicinski, saeedm, mlxsw, f.fainelli
In-Reply-To: <20190912112938.2292-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Listen to status notifications coming from kernel during flashing and
put them on stdout to inform user about the status.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- rebased (traps, pr_x conversion)
v2->v3:
- added example in man
v1->v2:
- fixed endless loop bug in case of no notifications
---
 devlink/devlink.c      | 215 ++++++++++++++++++++++++++++++++++++++++-
 devlink/mnlg.c         |   5 +
 devlink/mnlg.h         |   1 +
 man/man8/devlink-dev.8 |  11 +++
 4 files changed, 228 insertions(+), 4 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 31c319e3ef7a..00ebfb598515 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -25,6 +25,7 @@
 #include <linux/devlink.h>
 #include <libmnl/libmnl.h>
 #include <netinet/ether.h>
+#include <sys/types.h>
 
 #include "SNAPSHOT.h"
 #include "list.h"
@@ -96,6 +97,18 @@ pr_out_sp(unsigned int num, const char *fmt, ...)
 	g_new_line_count = 0;			\
 }
 
+static void __attribute__((format(printf, 1, 2)))
+pr_out_tty(const char *fmt, ...)
+{
+	va_list ap;
+
+	if (!isatty(STDOUT_FILENO))
+		return;
+	va_start(ap, fmt);
+	vprintf(fmt, ap);
+	va_end(ap);
+}
+
 static void __pr_out_indent_inc(void)
 {
 	if (g_indent_level + INDENT_STR_STEP > INDENT_STR_MAXLEN)
@@ -135,9 +148,8 @@ static int _mnlg_socket_recv_run(struct mnlg_socket *nlg,
 	return 0;
 }
 
-static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
-			       const struct nlmsghdr *nlh,
-			       mnl_cb_t data_cb, void *data)
+static int _mnlg_socket_send(struct mnlg_socket *nlg,
+			     const struct nlmsghdr *nlh)
 {
 	int err;
 
@@ -146,6 +158,18 @@ static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
 		pr_err("Failed to call mnlg_socket_send\n");
 		return -errno;
 	}
+	return 0;
+}
+
+static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
+			       const struct nlmsghdr *nlh,
+			       mnl_cb_t data_cb, void *data)
+{
+	int err;
+
+	err = _mnlg_socket_send(nlg, nlh);
+	if (err)
+		return err;
 	return _mnlg_socket_recv_run(nlg, data_cb, data);
 }
 
@@ -2830,9 +2854,151 @@ static void cmd_dev_flash_help(void)
 	pr_err("Usage: devlink dev flash DEV file PATH [ component NAME ]\n");
 }
 
+
+struct cmd_dev_flash_status_ctx {
+	struct dl *dl;
+	char *last_msg;
+	char *last_component;
+	uint8_t not_first:1,
+		last_pc:1,
+		received_end:1,
+		flash_done:1;
+};
+
+static int nullstrcmp(const char *str1, const char *str2)
+{
+	if (str1 && str2)
+		return strcmp(str1, str2);
+	if (!str1 && !str2)
+		return 0;
+	return str1 ? 1 : -1;
+}
+
+static int cmd_dev_flash_status_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct cmd_dev_flash_status_ctx *ctx = data;
+	struct dl_opts *opts = &ctx->dl->opts;
+	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+	const char *component = NULL;
+	uint64_t done = 0, total = 0;
+	const char *msg = NULL;
+	const char *bus_name;
+	const char *dev_name;
+
+	if (genl->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS &&
+	    genl->cmd != DEVLINK_CMD_FLASH_UPDATE_END)
+		return MNL_CB_STOP;
+
+	mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+	if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
+		return MNL_CB_ERROR;
+	bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
+	dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
+	if (strcmp(bus_name, opts->bus_name) ||
+	    strcmp(dev_name, opts->dev_name))
+		return MNL_CB_ERROR;
+
+	if (genl->cmd == DEVLINK_CMD_FLASH_UPDATE_END && ctx->not_first) {
+		pr_out("\n");
+		free(ctx->last_msg);
+		free(ctx->last_component);
+		ctx->received_end = 1;
+		return MNL_CB_STOP;
+	}
+
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG])
+		msg = mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG]);
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT])
+		component = mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT]);
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE])
+		done = mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE]);
+	if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL])
+		total = mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL]);
+
+	if (!nullstrcmp(msg, ctx->last_msg) &&
+	    !nullstrcmp(component, ctx->last_component) &&
+	    ctx->last_pc && ctx->not_first) {
+		pr_out_tty("\b\b\b\b\b"); /* clean percentage */
+	} else {
+		if (ctx->not_first)
+			pr_out("\n");
+		if (component) {
+			pr_out("[%s] ", component);
+			free(ctx->last_component);
+			ctx->last_component = strdup(component);
+		}
+		if (msg) {
+			pr_out("%s", msg);
+			free(ctx->last_msg);
+			ctx->last_msg = strdup(msg);
+		}
+	}
+	if (total) {
+		pr_out_tty(" %3lu%%", (done * 100) / total);
+		ctx->last_pc = 1;
+	} else {
+		ctx->last_pc = 0;
+	}
+	fflush(stdout);
+	ctx->not_first = 1;
+
+	return MNL_CB_STOP;
+}
+
+static int cmd_dev_flash_fds_process(struct cmd_dev_flash_status_ctx *ctx,
+				     struct mnlg_socket *nlg_ntf,
+				     int pipe_r)
+{
+	int nlfd = mnlg_socket_get_fd(nlg_ntf);
+	fd_set fds[3];
+	int fdmax;
+	int i;
+	int err;
+	int err2;
+
+	for (i = 0; i < 3; i++)
+		FD_ZERO(&fds[i]);
+	FD_SET(pipe_r, &fds[0]);
+	fdmax = pipe_r + 1;
+	FD_SET(nlfd, &fds[0]);
+	if (nlfd >= fdmax)
+		fdmax = nlfd + 1;
+
+	while (select(fdmax, &fds[0], &fds[1], &fds[2], NULL) < 0) {
+		if (errno == EINTR)
+			continue;
+		pr_err("select() failed\n");
+		return -errno;
+	}
+	if (FD_ISSET(nlfd, &fds[0])) {
+		err = _mnlg_socket_recv_run(nlg_ntf,
+					    cmd_dev_flash_status_cb, ctx);
+		if (err)
+			return err;
+	}
+	if (FD_ISSET(pipe_r, &fds[0])) {
+		err = read(pipe_r, &err2, sizeof(err2));
+		if (err == -1) {
+			pr_err("Failed to read pipe\n");
+			return -errno;
+		}
+		if (err2)
+			return err2;
+		ctx->flash_done = 1;
+	}
+	return 0;
+}
+
+
 static int cmd_dev_flash(struct dl *dl)
 {
+	struct cmd_dev_flash_status_ctx ctx = {.dl = dl,};
+	struct mnlg_socket *nlg_ntf;
 	struct nlmsghdr *nlh;
+	int pipe_r, pipe_w;
+	int pipe_fds[2];
+	pid_t pid;
 	int err;
 
 	if (dl_argv_match(dl, "help") || dl_no_arg(dl)) {
@@ -2848,7 +3014,48 @@ static int cmd_dev_flash(struct dl *dl)
 	if (err)
 		return err;
 
-	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+	nlg_ntf = mnlg_socket_open(DEVLINK_GENL_NAME, DEVLINK_GENL_VERSION);
+	if (!nlg_ntf)
+		return err;
+
+	err = _mnlg_socket_group_add(nlg_ntf, DEVLINK_GENL_MCGRP_CONFIG_NAME);
+	if (err)
+		return err;
+
+	err = pipe(pipe_fds);
+	if (err == -1)
+		return -errno;
+	pipe_r = pipe_fds[0];
+	pipe_w = pipe_fds[1];
+
+	pid = fork();
+	if (pid == -1) {
+		close(pipe_r);
+		close(pipe_w);
+		return -errno;
+	} else if (!pid) {
+		/* In child, just execute the flash and pass returned
+		 * value through pipe once it is done.
+		 */
+		close(pipe_r);
+		err = _mnlg_socket_send(dl->nlg, nlh);
+		write(pipe_w, &err, sizeof(err));
+		close(pipe_w);
+		exit(0);
+	}
+	close(pipe_w);
+
+	do {
+		err = cmd_dev_flash_fds_process(&ctx, nlg_ntf, pipe_r);
+		if (err)
+			goto out;
+	} while (!ctx.flash_done || (ctx.not_first && !ctx.received_end));
+
+	err = _mnlg_socket_recv_run(dl->nlg, NULL, NULL);
+out:
+	close(pipe_r);
+	mnlg_socket_close(nlg_ntf);
+	return err;
 }
 
 static int cmd_dev(struct dl *dl)
diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index ee125df042f0..c7d25e8713a1 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -320,3 +320,8 @@ void mnlg_socket_close(struct mnlg_socket *nlg)
 	free(nlg->buf);
 	free(nlg);
 }
+
+int mnlg_socket_get_fd(struct mnlg_socket *nlg)
+{
+	return mnl_socket_get_fd(nlg->nl);
+}
diff --git a/devlink/mnlg.h b/devlink/mnlg.h
index 4d1babf3b4c2..61bc5a3f31aa 100644
--- a/devlink/mnlg.h
+++ b/devlink/mnlg.h
@@ -23,5 +23,6 @@ int mnlg_socket_recv_run(struct mnlg_socket *nlg, mnl_cb_t data_cb, void *data);
 int mnlg_socket_group_add(struct mnlg_socket *nlg, const char *group_name);
 struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version);
 void mnlg_socket_close(struct mnlg_socket *nlg);
+int mnlg_socket_get_fd(struct mnlg_socket *nlg);
 
 #endif /* _MNLG_H_ */
diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
index 1804463b2321..1021ee8d064c 100644
--- a/man/man8/devlink-dev.8
+++ b/man/man8/devlink-dev.8
@@ -244,6 +244,17 @@ Sets the parameter internal_error_reset of specified devlink device to true.
 devlink dev reload pci/0000:01:00.0
 .RS 4
 Performs hot reload of specified devlink device.
+.RE
+.PP
+devlink dev flash pci/0000:01:00.0 file firmware.bin
+.RS 4
+Flashes the specified devlink device with provided firmware file name. If the driver supports it, user gets updates about the flash status. For example:
+.br
+Preparing to flash
+.br
+Flashing 100%
+.br
+Flashing done
 
 .SH SEE ALSO
 .BR devlink (8),
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net v2 01/11] net: core: limit nested device depth
From: David Miller @ 2019-09-12 11:37 UTC (permalink / raw)
  To: ap420073
  Cc: netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, sthemmin, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
In-Reply-To: <CAMArcTWMjTsZB8Ssx+hVMK-3-XozZw7AqVE62-H+zrJ+doC5Lw@mail.gmail.com>

From: Taehee Yoo <ap420073@gmail.com>
Date: Thu, 12 Sep 2019 19:14:37 +0900

> On Thu, 12 Sep 2019 at 18:38, David Miller <davem@davemloft.net> wrote:
>>
>> From: Taehee Yoo <ap420073@gmail.com>
>> Date: Thu, 12 Sep 2019 12:56:19 +0900
>>
>> > I tested with this reproducer commands without lockdep.
>> >
>> >     ip link add dummy0 type dummy
>> >     ip link add link dummy0 name vlan1 type vlan id 1
>> >     ip link set vlan1 up
>> >
>> >     for i in {2..200}
>> >     do
>> >             let A=$i-1
>> >
>> >             ip link add name vlan$i link vlan$A type vlan id $i
>> >     done
>> >     ip link del vlan1 <-- this command is added.
>>
>> Is there any other device type which allows arbitrary nesting depth
>> in this manner other than VLAN?  Perhaps it is the VLAN nesting
>> depth that we should limit instead of all of this extra code.
> 
> Below device types have the same problem.
> VLAN, BONDING, TEAM, VXLAN, MACVLAN, and MACSEC.
> All the below test commands reproduce a panic.

I think then we need to move the traversals over to a iterative
rather than recursive algorithm.

^ permalink raw reply

* Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware operations for PTP
From: David Miller @ 2019-09-12 11:37 UTC (permalink / raw)
  To: olteanv; +Cc: f.fainelli, vivien.didelot, andrew, richardcochran, netdev
In-Reply-To: <CA+h21hoBQ=4pSCgwcYWErA7k7BQ02LMun_qZ476-bB4eY6RjjQ@mail.gmail.com>

From: Vladimir Oltean <olteanv@gmail.com>
Date: Thu, 12 Sep 2019 11:17:11 +0100

> Hi Dave,
> 
> On 12/09/2019, David Miller <davem@davemloft.net> wrote:
>> From: Vladimir Oltean <olteanv@gmail.com>
>> Date: Tue, 10 Sep 2019 04:34:57 +0300
>>
>>>  static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long
>>> scaled_ppm)
>>>  {
>>>  	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>>> +	const struct sja1105_regs *regs = priv->info->regs;
>>>  	s64 clkrate;
>>> +	int rc;
>>  ..
>>> -static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>>> -{
>>> -	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>>> +	rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
>>> +				  &clkrate, 4);
>>
>> You're sending an arbitrary 4 bytes of a 64-bit value.  This works on little
>> endian
>> but will not on big endian.
>>
>> Please properly copy this clkrate into a "u32" variable and pass that into
>> sja1105_spi_send_int().
>>
>> It also seems to suggest that you want to use abs() to perform that weird
>> centering around 1 << 31 calculation.
>>
>> Thank you.
>>
> 
> It looks 'wrong' but it isn't. The driver uses the 'packing' framework
> (lib/packing.c) which is endian-agnostic (converts between CPU and
> peripheral endianness) and operates on u64 as the CPU word size. On
> the contrary, u32 would not work with the 'packing' API in its current
> form, but I don't see yet any reasons to extend it (packing64,
> packing32 etc).

That's extremely unintuitive and makes auditing patches next to impossible.

^ permalink raw reply

* Re: [Patch net] sch_sfb: fix a crash in sfb_destroy()
From: David Miller @ 2019-09-12 11:42 UTC (permalink / raw)
  To: torvalds
  Cc: xiyou.wangcong, eric.dumazet, netdev, syzbot+d5870a903591faaca4ae,
	jhs, jiri
In-Reply-To: <CAHk-=whO37+O-mohvMODnD57ppCsK3Bcv8oHzSBvmwJbsT54cA@mail.gmail.com>

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 12 Sep 2019 11:31:06 +0100

> It depends on what you want to do, of course. Do you want to make sure
> each user is being very careful? Or do you want to make the interfaces
> easy to use without _having_ to be careful? There are arguments both
> ways, but we've tended to move more towards a "easy to use" model than
> the "be careful" one.

Yes, I think allowing NULL or error pointers on free/destroy makes sense.

^ permalink raw reply

* Re: [PATCH] ixgbe: Fix secpath usage for IPsec TX offload.
From: David Miller @ 2019-09-12 11:43 UTC (permalink / raw)
  To: steffen.klassert
  Cc: jeffrey.t.kirsher, intel-wired-lan, michael, snelson, netdev
In-Reply-To: <20190912110144.GS2879@gauss3.secunet.de>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 12 Sep 2019 13:01:44 +0200

> The ixgbe driver currently does IPsec TX offloading
> based on an existing secpath. However, the secpath
> can also come from the RX side, in this case it is
> misinterpreted for TX offload and the packets are
> dropped with a "bad sa_idx" error. Fix this by using
> the xfrm_offload() function to test for TX offload.
> 
> Fixes: 592594704761 ("ixgbe: process the Tx ipsec offload")
> Reported-by: Michael Marley <michael@michaelmarley.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

I'll apply this directly and queue it up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH] cxgb4: Fix spelling typos
From: David Miller @ 2019-09-12 11:51 UTC (permalink / raw)
  To: arkadiusz; +Cc: vishal, netdev, linux-kernel
In-Reply-To: <20190910204901.11741-1-arkadiusz@drabczyk.org>

From: Arkadiusz Drabczyk <arkadiusz@drabczyk.org>
Date: Tue, 10 Sep 2019 22:49:01 +0200

> Fix several spelling typos in comments in t4_hw.c.
> 
> Signed-off-by: Arkadiusz Drabczyk <arkadiusz@drabczyk.org>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH net v2 01/11] net: core: limit nested device depth
From: Taehee Yoo @ 2019-09-12 11:54 UTC (permalink / raw)
  To: David Miller
  Cc: Netdev, j.vosburgh, vfalico, Andy Gospodarek,
	Jiří Pírko, sd, Roopa Prabhu, saeedm, manishc,
	rahulv, kys, haiyangz, sthemmin, sashal, hare, varun, ubraun,
	kgraul, Jay Vosburgh
In-Reply-To: <20190912.133717.257813019167130934.davem@davemloft.net>

On Thu, 12 Sep 2019 at 20:37, David Miller <davem@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420073@gmail.com>
> Date: Thu, 12 Sep 2019 19:14:37 +0900
>
> > On Thu, 12 Sep 2019 at 18:38, David Miller <davem@davemloft.net> wrote:
> >>
> >> From: Taehee Yoo <ap420073@gmail.com>
> >> Date: Thu, 12 Sep 2019 12:56:19 +0900
> >>
> >> > I tested with this reproducer commands without lockdep.
> >> >
> >> >     ip link add dummy0 type dummy
> >> >     ip link add link dummy0 name vlan1 type vlan id 1
> >> >     ip link set vlan1 up
> >> >
> >> >     for i in {2..200}
> >> >     do
> >> >             let A=$i-1
> >> >
> >> >             ip link add name vlan$i link vlan$A type vlan id $i
> >> >     done
> >> >     ip link del vlan1 <-- this command is added.
> >>
> >> Is there any other device type which allows arbitrary nesting depth
> >> in this manner other than VLAN?  Perhaps it is the VLAN nesting
> >> depth that we should limit instead of all of this extra code.
> >
> > Below device types have the same problem.
> > VLAN, BONDING, TEAM, VXLAN, MACVLAN, and MACSEC.
> > All the below test commands reproduce a panic.
>
> I think then we need to move the traversals over to a iterative
> rather than recursive algorithm.

I agree with that.
So I will check it out then send a v3 patchset.

Thank you!

^ permalink raw reply

* Re: [PATCH] sctp: Fix the link time qualifier of 'sctp_ctrlsock_exit()'
From: David Miller @ 2019-09-12 11:56 UTC (permalink / raw)
  To: christophe.jaillet
  Cc: vyasevich, nhorman, marcelo.leitner, linux-sctp, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <20190911160239.10734-1-christophe.jaillet@wanadoo.fr>

From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Wed, 11 Sep 2019 18:02:39 +0200

> The '.exit' functions from 'pernet_operations' structure should be marked
> as __net_exit, not __net_init.
> 
> Fixes: 8e2d61e0aed2 ("sctp: fix race on protocol/netns initialization")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware operations for PTP
From: Vladimir Oltean @ 2019-09-12 11:58 UTC (permalink / raw)
  To: David Miller; +Cc: f.fainelli, vivien.didelot, andrew, richardcochran, netdev
In-Reply-To: <20190912.133753.1473374980190418320.davem@davemloft.net>

On 12/09/2019, David Miller <davem@davemloft.net> wrote:
> From: Vladimir Oltean <olteanv@gmail.com>
> Date: Thu, 12 Sep 2019 11:17:11 +0100
>
>> Hi Dave,
>>
>> On 12/09/2019, David Miller <davem@davemloft.net> wrote:
>>> From: Vladimir Oltean <olteanv@gmail.com>
>>> Date: Tue, 10 Sep 2019 04:34:57 +0300
>>>
>>>>  static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long
>>>> scaled_ppm)
>>>>  {
>>>>  	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>>>> +	const struct sja1105_regs *regs = priv->info->regs;
>>>>  	s64 clkrate;
>>>> +	int rc;
>>>  ..
>>>> -static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>>>> -{
>>>> -	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>>>> +	rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
>>>> +				  &clkrate, 4);
>>>
>>> You're sending an arbitrary 4 bytes of a 64-bit value.  This works on
>>> little
>>> endian
>>> but will not on big endian.
>>>
>>> Please properly copy this clkrate into a "u32" variable and pass that
>>> into
>>> sja1105_spi_send_int().
>>>
>>> It also seems to suggest that you want to use abs() to perform that
>>> weird
>>> centering around 1 << 31 calculation.
>>>
>>> Thank you.
>>>
>>
>> It looks 'wrong' but it isn't. The driver uses the 'packing' framework
>> (lib/packing.c) which is endian-agnostic (converts between CPU and
>> peripheral endianness) and operates on u64 as the CPU word size. On
>> the contrary, u32 would not work with the 'packing' API in its current
>> form, but I don't see yet any reasons to extend it (packing64,
>> packing32 etc).
>
> That's extremely unintuitive and makes auditing patches next to impossible.
>

Through static analysis maybe you're right - I don't yet grasp what it
takes to prove an API is used correctly at build time, but I can look
at how others are doing it.
At runtime, there is sanity checking throughout and all the bugs I've
had while calling packing() incorrectly I caught them right away.
spi_send_int in particular is just a wrapper for packing an N byte
sized word (which fits in u64) in bits [8*N-1, 0] of the buffer, as
per peripheral memory ordering quirks. This is perhaps the trivial
case that can be handled through other APIs as well, but there are
times when I need to pack an u64 into a bit field that crosses even
64-bit boundaries. Combine that with weird byte ordering, and the
sja1105 driver would have simply not existed if it had to open-code
all of that. The API's high-level goal is for readers to be able to
follow along smoothly with the register manual.
All I'm saying is that I'm willing to make the packing API more
sane/checkable, but at the moment I just don't see the better
alternative.

Thanks,
-Vladimir

^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Jiri Pirko @ 2019-09-12 11:59 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: Michal Kubecek, netdev, David Miller, Jakub Kicinski, David Ahern,
	Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
	mlxsw
In-Reply-To: <20190830170342.GR2312@nanopsycho>

Fri, Aug 30, 2019 at 07:03:42PM CEST, jiri@resnulli.us wrote:
>Fri, Aug 30, 2019 at 04:35:23PM CEST, roopa@cumulusnetworks.com wrote:

[...]

>>
>>so to summarize, i think we have discussed the following options to
>>update a netlink list attribute so far:
>>(a) encode an optional attribute/flag in the list attribute in
>>RTM_SETLINK to indicate if it is a add or del
>>(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
>>(close to bridge vlan add/del)
>
>Nope, bridge vlan add/del is done according to the cmd, not any flag.
>
>
>>(c) introduce a separate generic msg type to add/del to a list
>>attribute (IIUC this does need a separate msg type per subsystem or
>>netlink API)

Getting back to this, sorry.

Thinking about it for some time, a,b,c have all their issues. Why can't
we have another separate cmd as I originally proposed in this RFC? Does
anyone have any argument against it? Could you please describe?

Because otherwise, I don't feel comfortable going to any of a,b,c :(

Thanks!


^ permalink raw reply

* Re: [PATCH net v2 01/11] net: core: limit nested device depth
From: Taehee Yoo @ 2019-09-12 12:49 UTC (permalink / raw)
  To: David Miller
  Cc: Netdev, j.vosburgh, vfalico, Andy Gospodarek,
	Jiří Pírko, sd, Roopa Prabhu, saeedm, manishc,
	rahulv, kys, haiyangz, sthemmin, sashal, hare, varun, ubraun,
	kgraul, Jay Vosburgh
In-Reply-To: <20190912.133717.257813019167130934.davem@davemloft.net>

On Thu, 12 Sep 2019 at 20:37, David Miller <davem@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420073@gmail.com>
> Date: Thu, 12 Sep 2019 19:14:37 +0900
>
> > On Thu, 12 Sep 2019 at 18:38, David Miller <davem@davemloft.net> wrote:
> >>
> >> From: Taehee Yoo <ap420073@gmail.com>
> >> Date: Thu, 12 Sep 2019 12:56:19 +0900
> >>
> >> > I tested with this reproducer commands without lockdep.
> >> >
> >> >     ip link add dummy0 type dummy
> >> >     ip link add link dummy0 name vlan1 type vlan id 1
> >> >     ip link set vlan1 up
> >> >
> >> >     for i in {2..200}
> >> >     do
> >> >             let A=$i-1
> >> >
> >> >             ip link add name vlan$i link vlan$A type vlan id $i
> >> >     done
> >> >     ip link del vlan1 <-- this command is added.
> >>
> >> Is there any other device type which allows arbitrary nesting depth
> >> in this manner other than VLAN?  Perhaps it is the VLAN nesting
> >> depth that we should limit instead of all of this extra code.
> >
> > Below device types have the same problem.
> > VLAN, BONDING, TEAM, VXLAN, MACVLAN, and MACSEC.
> > All the below test commands reproduce a panic.
>
> I think then we need to move the traversals over to a iterative
> rather than recursive algorithm.

Just to clarify, I have a question.

There are two recursive routines in the code.
a) netdev_walk_all_{lower/upper}_dev() that are used to traversal
all of their lower or upper devices.
b) VLAN, BONDING, TEAM, VXLAN, MACVLAN, and MACSEC
modules internally handle their lower/upper devices recursively
when an event is received such as unregistering.

what is the routine that you mentioned?

^ permalink raw reply

* [PATCH net-next 0/2] mlxsw: spectrum_buffers: Add the ability to query the CPU port's shared buffer
From: Ido Schimmel @ 2019-09-12 13:07 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Shalom says:

While debugging packet loss towards the CPU, it is useful to be able to
query the CPU port's shared buffer quotas and occupancy.

Patch #1 registers the CPU port with devlink.

Patch #2 adds the ability to query the CPU port's shared buffer quotas and
occupancy.

Shalom Toledo (2):
  mlxsw: spectrum: Register CPU port with devlink
  mlxsw: spectrum_buffers: Add the ability to query the CPU port's
    shared buffer

 drivers/net/ethernet/mellanox/mlxsw/core.c    | 33 ++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  5 ++
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 47 +++++++++++++++++
 .../mellanox/mlxsw/spectrum_buffers.c         | 51 ++++++++++++++++---
 4 files changed, 128 insertions(+), 8 deletions(-)

-- 
2.21.0


^ permalink raw reply

* [PATCH net-next 1/2] mlxsw: spectrum: Register CPU port with devlink
From: Ido Schimmel @ 2019-09-12 13:07 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, shalomt, mlxsw, Ido Schimmel
In-Reply-To: <20190912130740.22061-1-idosch@idosch.org>

From: Shalom Toledo <shalomt@mellanox.com>

Register CPU port with devlink.

Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c    | 33 +++++++++++++
 drivers/net/ethernet/mellanox/mlxsw/core.h    |  5 ++
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 47 +++++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 963a2b4b61b1..94f83d2be17e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1892,6 +1892,39 @@ void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
 }
 EXPORT_SYMBOL(mlxsw_core_port_fini);
 
+int mlxsw_core_cpu_port_init(struct mlxsw_core *mlxsw_core,
+			     void *port_driver_priv,
+			     const unsigned char *switch_id,
+			     unsigned char switch_id_len)
+{
+	struct devlink *devlink = priv_to_devlink(mlxsw_core);
+	struct mlxsw_core_port *mlxsw_core_port =
+					&mlxsw_core->ports[MLXSW_PORT_CPU_PORT];
+	struct devlink_port *devlink_port = &mlxsw_core_port->devlink_port;
+	int err;
+
+	mlxsw_core_port->local_port = MLXSW_PORT_CPU_PORT;
+	mlxsw_core_port->port_driver_priv = port_driver_priv;
+	devlink_port_attrs_set(devlink_port, DEVLINK_PORT_FLAVOUR_CPU,
+			       0, false, 0, switch_id, switch_id_len);
+	err = devlink_port_register(devlink, devlink_port, MLXSW_PORT_CPU_PORT);
+	if (err)
+		memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
+	return err;
+}
+EXPORT_SYMBOL(mlxsw_core_cpu_port_init);
+
+void mlxsw_core_cpu_port_fini(struct mlxsw_core *mlxsw_core)
+{
+	struct mlxsw_core_port *mlxsw_core_port =
+					&mlxsw_core->ports[MLXSW_PORT_CPU_PORT];
+	struct devlink_port *devlink_port = &mlxsw_core_port->devlink_port;
+
+	devlink_port_unregister(devlink_port);
+	memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
+}
+EXPORT_SYMBOL(mlxsw_core_cpu_port_fini);
+
 void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 			     void *port_driver_priv, struct net_device *dev)
 {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index b65a17d49e43..5d7d2ab6d155 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -177,6 +177,11 @@ int mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
 			 const unsigned char *switch_id,
 			 unsigned char switch_id_len);
 void mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port);
+int mlxsw_core_cpu_port_init(struct mlxsw_core *mlxsw_core,
+			     void *port_driver_priv,
+			     const unsigned char *switch_id,
+			     unsigned char switch_id_len);
+void mlxsw_core_cpu_port_fini(struct mlxsw_core *mlxsw_core);
 void mlxsw_core_port_eth_set(struct mlxsw_core *mlxsw_core, u8 local_port,
 			     void *port_driver_priv, struct net_device *dev);
 void mlxsw_core_port_ib_set(struct mlxsw_core *mlxsw_core, u8 local_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 91e4792bb7e7..1fc73a9ad84d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3872,6 +3872,46 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	mlxsw_core_port_fini(mlxsw_sp->core, local_port);
 }
 
+static int mlxsw_sp_cpu_port_create(struct mlxsw_sp *mlxsw_sp)
+{
+	struct mlxsw_sp_port *mlxsw_sp_port;
+	int err;
+
+	mlxsw_sp_port = kzalloc(sizeof(*mlxsw_sp_port), GFP_KERNEL);
+	if (!mlxsw_sp_port)
+		return -ENOMEM;
+
+	mlxsw_sp_port->mlxsw_sp = mlxsw_sp;
+	mlxsw_sp_port->local_port = MLXSW_PORT_CPU_PORT;
+
+	mlxsw_sp->ports[MLXSW_PORT_CPU_PORT] = mlxsw_sp_port;
+
+	err = mlxsw_core_cpu_port_init(mlxsw_sp->core,
+				       mlxsw_sp_port,
+				       mlxsw_sp->base_mac,
+				       sizeof(mlxsw_sp->base_mac));
+	if (err) {
+		dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize core CPU port\n");
+		goto err_core_cpu_port_init;
+	}
+
+	return err;
+
+err_core_cpu_port_init:
+	mlxsw_sp->ports[MLXSW_PORT_CPU_PORT] = NULL;
+	kfree(mlxsw_sp_port);
+	return err;
+}
+
+static void mlxsw_sp_cpu_port_remove(struct mlxsw_sp *mlxsw_sp)
+{
+	struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[0];
+
+	mlxsw_core_cpu_port_fini(mlxsw_sp->core);
+	mlxsw_sp->ports[MLXSW_PORT_CPU_PORT] = NULL;
+	kfree(mlxsw_sp_port);
+}
+
 static bool mlxsw_sp_port_created(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 {
 	return mlxsw_sp->ports[local_port] != NULL;
@@ -3884,6 +3924,7 @@ static void mlxsw_sp_ports_remove(struct mlxsw_sp *mlxsw_sp)
 	for (i = 1; i < mlxsw_core_max_ports(mlxsw_sp->core); i++)
 		if (mlxsw_sp_port_created(mlxsw_sp, i))
 			mlxsw_sp_port_remove(mlxsw_sp, i);
+	mlxsw_sp_cpu_port_remove(mlxsw_sp);
 	kfree(mlxsw_sp->port_to_module);
 	kfree(mlxsw_sp->ports);
 }
@@ -3908,6 +3949,10 @@ static int mlxsw_sp_ports_create(struct mlxsw_sp *mlxsw_sp)
 		goto err_port_to_module_alloc;
 	}
 
+	err = mlxsw_sp_cpu_port_create(mlxsw_sp);
+	if (err)
+		goto err_cpu_port_create;
+
 	for (i = 1; i < max_ports; i++) {
 		/* Mark as invalid */
 		mlxsw_sp->port_to_module[i] = -1;
@@ -3931,6 +3976,8 @@ static int mlxsw_sp_ports_create(struct mlxsw_sp *mlxsw_sp)
 	for (i--; i >= 1; i--)
 		if (mlxsw_sp_port_created(mlxsw_sp, i))
 			mlxsw_sp_port_remove(mlxsw_sp, i);
+	mlxsw_sp_cpu_port_remove(mlxsw_sp);
+err_cpu_port_create:
 	kfree(mlxsw_sp->port_to_module);
 err_port_to_module_alloc:
 	kfree(mlxsw_sp->ports);
-- 
2.21.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox