* Re: [PATCH net-next v8 2/2] net: ethernet: Add driver for Sunplus SP7021
From: Andrew Lunn @ 2022-04-19 13:03 UTC (permalink / raw)
To: Wells Lu 呂芳騰
Cc: Jakub Kicinski, Wells Lu, davem@davemloft.net, robh+dt@kernel.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, p.zabel@pengutronix.de,
pabeni@redhat.com, krzk+dt@kernel.org, roopa@nvidia.com,
edumazet@google.com
In-Reply-To: <e784ab5356aa4b6e93765b54bdefea0a@sphcmbx02.sunplus.com.tw>
On Tue, Apr 19, 2022 at 10:07:55AM +0000, Wells Lu 呂芳騰 wrote:
> > > > + /* Get mac-address from nvmem. */
> > > > + ret = spl2sw_nvmem_get_mac_address(&pdev->dev, port_np, mac_addr);
> > > > + if (ret) {
> > > > + dev_info(&pdev->dev, "Generate a random mac address!\n");
> > > > +
> > > > + /* Generate a mac address using OUI of Sunplus Technology
> > > > + * and random controller number.
> > > > + */
> > > > + mac_addr[0] = 0xfc; /* OUI of Sunplus: fc:4b:bc */
> > > > + mac_addr[1] = 0x4b;
> > > > + mac_addr[2] = 0xbc;
> > > > + mac_addr[3] = get_random_int() % 256;
> > > > + mac_addr[4] = get_random_int() % 256;
> > > > + mac_addr[5] = get_random_int() % 256;
> > >
> > > I don't think you can do that. Either you use your OUI and assign the
> > > address at manufacture or you must use a locally administered address.
> > > And if locally administered address is used it better be completely
> > > random to lower the probability of collision to absolute minimum.
> >
> > I commented about that in an earlier version of these patches. We probably need a quote
> > from the 802.1 or 802.3 which says this is O.K.
> >
> > Andrew
>
> Hi Andrew,
>
> I plan to replace above statements with:
>
> eth_random_addr(mac_addr);
O.K, that is good.
> Do you mean I can keep use the mac address: "OUI + random number"?
If you can show us text in an IEEE 802.1, IEEE 802.3, or some other
IEEE document which says this is allowed.
> Only need to add comment for it.
Add a comment which points to a document which says you are allowed to
do this. This is very unusual, so questions will be asked, and if you
point people at the answer it will help.
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: Support riscv USDT argument parsing logic
From: Pu Lehui @ 2022-04-19 13:00 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, open list, Networking, linux-riscv, Andrii Nakryiko,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, john fastabend, KP Singh, Paul Walmsley,
Palmer Dabbelt, Albert Ou
In-Reply-To: <CAEf4BzbavSC=JN=sowFY3t4yOUfe8QtVXhdG+y7a-T1YtfRqXQ@mail.gmail.com>
On 2022/4/19 12:33, Andrii Nakryiko wrote:
> On Sun, Apr 17, 2022 at 8:53 PM Pu Lehui <pulehui@huawei.com> wrote:
>>
>> Add riscv-specific USDT argument specification parsing logic.
>> riscv USDT argument format is shown below:
>> - Memory dereference case:
>> "size@off(reg)", e.g. "-8@-88(s0)"
>> - Constant value case:
>> "size@val", e.g. "4@5"
>> - Register read case:
>> "size@reg", e.g. "-8@a1"
>>
>> s8 will be marked as poison while it's a reg of riscv, we need
>> to alias it in advance.
>>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>
> Can you please mention briefly the testing you performed as I'm not
> able to test this locally.
>
Both RV32 and RV64 have been tested. I will attach the test result in
v2. Meanwhile, I found a small problem with libbpf USDT, and will be
post in v2.
>> tools/lib/bpf/usdt.c | 107 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 107 insertions(+)
>>
>> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
>> index 934c25301ac1..b8af409cc763 100644
>> --- a/tools/lib/bpf/usdt.c
>> +++ b/tools/lib/bpf/usdt.c
>> @@ -10,6 +10,11 @@
>> #include <linux/ptrace.h>
>> #include <linux/kernel.h>
>>
>> +/* s8 will be marked as poison while it's a reg of riscv */
>> +#if defined(__riscv)
>> +#define rv_s8 s8
>> +#endif
>> +
>> #include "bpf.h"
>> #include "libbpf.h"
>> #include "libbpf_common.h"
>> @@ -1400,6 +1405,108 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
>> return len;
>> }
>>
>> +#elif defined(__riscv)
>> +
>> +static int calc_pt_regs_off(const char *reg_name)
>> +{
>> + static struct {
>> + const char *name;
>> + size_t pt_regs_off;
>> + } reg_map[] = {
>> + { "ra", offsetof(struct user_regs_struct, ra) },
>> + { "sp", offsetof(struct user_regs_struct, sp) },
>> + { "gp", offsetof(struct user_regs_struct, gp) },
>> + { "tp", offsetof(struct user_regs_struct, tp) },
>> + { "t0", offsetof(struct user_regs_struct, t0) },
>> + { "t1", offsetof(struct user_regs_struct, t1) },
>> + { "t2", offsetof(struct user_regs_struct, t2) },
>> + { "s0", offsetof(struct user_regs_struct, s0) },
>> + { "s1", offsetof(struct user_regs_struct, s1) },
>> + { "a0", offsetof(struct user_regs_struct, a0) },
>> + { "a1", offsetof(struct user_regs_struct, a1) },
>> + { "a2", offsetof(struct user_regs_struct, a2) },
>> + { "a3", offsetof(struct user_regs_struct, a3) },
>> + { "a4", offsetof(struct user_regs_struct, a4) },
>> + { "a5", offsetof(struct user_regs_struct, a5) },
>> + { "a6", offsetof(struct user_regs_struct, a6) },
>> + { "a7", offsetof(struct user_regs_struct, a7) },
>> + { "s2", offsetof(struct user_regs_struct, s2) },
>> + { "s3", offsetof(struct user_regs_struct, s3) },
>> + { "s4", offsetof(struct user_regs_struct, s4) },
>> + { "s5", offsetof(struct user_regs_struct, s5) },
>> + { "s6", offsetof(struct user_regs_struct, s6) },
>> + { "s7", offsetof(struct user_regs_struct, s7) },
>> + { "s8", offsetof(struct user_regs_struct, rv_s8) },
>> + { "s9", offsetof(struct user_regs_struct, s9) },
>> + { "s10", offsetof(struct user_regs_struct, s10) },
>> + { "s11", offsetof(struct user_regs_struct, s11) },
>> + { "t3", offsetof(struct user_regs_struct, t3) },
>> + { "t4", offsetof(struct user_regs_struct, t4) },
>> + { "t5", offsetof(struct user_regs_struct, t5) },
>> + { "t6", offsetof(struct user_regs_struct, t6) },
>
> would it make sense to order registers a bit more "logically"? Like
> s0-s11, t0-t6, etc. Right now it looks very random and it's hard to
> see if all the registers from some range of registers are defined.
>
I code it according to the RISCV specification, and for sure, we can
make it more intuitive.
Thanks,
Lehui
>> + };
>> + int i;
>> +
>
> [...]
> .
>
^ permalink raw reply
* Re: [PATCH net-next 00/17] Introduce line card support for modular switch
From: Ido Schimmel @ 2022-04-19 12:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, Ido Schimmel, netdev, davem, kuba, pabeni, jiri,
vadimp, petrm, andrew, mlxsw
In-Reply-To: <Yl6jQHXYa8MvEyX3@nanopsycho>
On Tue, Apr 19, 2022 at 01:55:44PM +0200, Jiri Pirko wrote:
> Mon, Apr 18, 2022 at 04:31:30PM CEST, dsahern@gmail.com wrote:
> >On 4/18/22 12:42 AM, Ido Schimmel wrote:
> >> Jiri says:
> >>
> >> This patchset introduces support for modular switch systems and also
> >> introduces mlxsw support for NVIDIA Mellanox SN4800 modular switch.
> >> It contains 8 slots to accommodate line cards - replaceable PHY modules
> >> which may contain gearboxes.
> >> Currently supported line card:
> >> 16X 100GbE (QSFP28)
> >> Other line cards that are going to be supported:
> >> 8X 200GbE (QSFP56)
> >> 4X 400GbE (QSFP-DD)
> >> There may be other types of line cards added in the future.
> >>
> >> To be consistent with the port split configuration (splitter cabels),
> >> the line card entities are treated in the similar way. The nature of
> >> a line card is not "a pluggable device", but "a pluggable PHY module".
> >>
> >> A concept of "provisioning" is introduced. The user may "provision"
> >> certain slot with a line card type. Driver then creates all instances
> >> (devlink ports, netdevices, etc) related to this line card type. It does
> >> not matter if the line card is plugged-in at the time. User is able to
> >> configure netdevices, devlink ports, setup port splitters, etc. From the
> >> perspective of the switch ASIC, all is present and can be configured.
> >>
> >> The carrier of netdevices stays down if the line card is not plugged-in.
> >> Once the line card is inserted and activated, the carrier of
> >> the related netdevices is then reflecting the physical line state,
> >> same as for an ordinary fixed port.
> >>
> >> Once user does not want to use the line card related instances
> >> anymore, he can "unprovision" the slot. Driver then removes the
> >> instances.
> >>
> >> Patches 1-4 are extending devlink driver API and UAPI in order to
> >> register, show, dump, provision and activate the line card.
> >> Patches 5-17 are implementing the introduced API in mlxsw.
> >> The last patch adds a selftest for mlxsw line cards.
> >>
> >> Example:
> >> $ devlink port # No ports are listed
> >> $ devlink lc
> >> pci/0000:01:00.0:
> >> lc 1 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 2 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 3 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 4 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 5 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 6 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 7 state unprovisioned
> >> supported_types:
> >> 16x100G
> >> lc 8 state unprovisioned
> >> supported_types:
> >> 16x100G
> >>
> >> Note that driver exposes list supported line card types. Currently
> >> there is only one: "16x100G".
> >>
> >> To provision the slot #8:
> >>
> >> $ devlink lc set pci/0000:01:00.0 lc 8 type 16x100G
> >> $ devlink lc show pci/0000:01:00.0 lc 8
> >> pci/0000:01:00.0:
> >> lc 8 state active type 16x100G
> >> supported_types:
> >> 16x100G
> >> $ devlink port
> >> pci/0000:01:00.0/0: type notset flavour cpu port 0 splittable false
> >> pci/0000:01:00.0/53: type eth netdev enp1s0nl8p1 flavour physical lc 8 port 1 splittable true lanes 4
> >> pci/0000:01:00.0/54: type eth netdev enp1s0nl8p2 flavour physical lc 8 port 2 splittable true lanes 4
> >> pci/0000:01:00.0/55: type eth netdev enp1s0nl8p3 flavour physical lc 8 port 3 splittable true lanes 4
> >> pci/0000:01:00.0/56: type eth netdev enp1s0nl8p4 flavour physical lc 8 port 4 splittable true lanes 4
> >> pci/0000:01:00.0/57: type eth netdev enp1s0nl8p5 flavour physical lc 8 port 5 splittable true lanes 4
> >> pci/0000:01:00.0/58: type eth netdev enp1s0nl8p6 flavour physical lc 8 port 6 splittable true lanes 4
> >> pci/0000:01:00.0/59: type eth netdev enp1s0nl8p7 flavour physical lc 8 port 7 splittable true lanes 4
> >> pci/0000:01:00.0/60: type eth netdev enp1s0nl8p8 flavour physical lc 8 port 8 splittable true lanes 4
> >> pci/0000:01:00.0/61: type eth netdev enp1s0nl8p9 flavour physical lc 8 port 9 splittable true lanes 4
> >> pci/0000:01:00.0/62: type eth netdev enp1s0nl8p10 flavour physical lc 8 port 10 splittable true lanes 4
> >> pci/0000:01:00.0/63: type eth netdev enp1s0nl8p11 flavour physical lc 8 port 11 splittable true lanes 4
> >> pci/0000:01:00.0/64: type eth netdev enp1s0nl8p12 flavour physical lc 8 port 12 splittable true lanes 4
> >> pci/0000:01:00.0/125: type eth netdev enp1s0nl8p13 flavour physical lc 8 port 13 splittable true lanes 4
> >> pci/0000:01:00.0/126: type eth netdev enp1s0nl8p14 flavour physical lc 8 port 14 splittable true lanes 4
> >> pci/0000:01:00.0/127: type eth netdev enp1s0nl8p15 flavour physical lc 8 port 15 splittable true lanes 4
> >> pci/0000:01:00.0/128: type eth netdev enp1s0nl8p16 flavour physical lc 8 port 16 splittable true lanes 4
> >>
> >> To uprovision the slot #8:
> >>
> >> $ devlink lc set pci/0000:01:00.0 lc 8 notype
> >>
> >
> >are there any changes from the last RFC?
> >
> >https://lore.kernel.org/netdev/20210122094648.1631078-1-jiri@resnulli.us/
>
> Yes, many of them, I din't track them. Mainly, the RFC was backed by
> netdevsim implementation, this is mlxsw with actual HW underneath.
I don't think David was asking about the backing implementation, but
about the interface itself. I don't remember any changes in this aspect
during the internal review and I also compared the uAPI files and they
seem to be the same. The only change that I do remember is making the
APIs idempotent. Previously, this would fail:
# devlink lc set pci/0000:01:00.0 lc 8 notype
# devlink lc set pci/0000:01:00.0 lc 8 notype
^ permalink raw reply
* Re: [PATCH net-next 09/12] ARM: dts: r9a06g032: describe MII converter
From: Andrew Lunn @ 2022-04-19 12:57 UTC (permalink / raw)
To: Clément Léger
Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Geert Uytterhoeven, Magnus Damm,
Heiner Kallweit, Russell King, Thomas Petazzoni, Herve Codina,
Miquèl Raynal, Milan Stevanovic, Jimmy Lalande, linux-kernel,
devicetree, linux-renesas-soc, netdev
In-Reply-To: <20220419110328.0241fb1f@fixe.home>
> Hum, that could be done but since only some values/combinations are
> allowed, it would potentially require to validate the setting at each
> request, leading to potential non working devices due to invalid MUX
> configuration required.
Yes, validation is messy, you have to incrementally validate as each
device probes and requests its PCS. I would not only return -EINVAL,
but also dump the current partial configuration to the kernel log. I
guess the implementation would have a big table as shown in the
datasheet. You walk the table trying to find a match for those
settings you have so far, and wildcard those you don't know yet. Fun
little coding problem.
Andrew
^ permalink raw reply
* [PATCH ethtool-next] ethtool: netlink: add support to get/set tx push by ethtool -G/g
From: Guangbin Huang @ 2022-04-19 12:50 UTC (permalink / raw)
To: mkubecek, davem, kuba; +Cc: netdev, lipeng321, huangguangbin2
From: Jie Wang <wangjie125@huawei.com>
Currently tx push is a standard feature for NICs such as Mellanox, HNS3.
But there is no command to set or get this feature.
So this patch adds support for "ethtool -G <dev> tx-push on|off" and
"ethtool -g <dev>" to set/get tx push mode.
Signed-off-by: Jie Wang <wangjie125@huawei.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
ethtool.8.in | 4 ++++
ethtool.c | 1 +
netlink/rings.c | 7 +++++++
uapi/linux/ethtool_netlink.h | 1 +
4 files changed, 13 insertions(+)
diff --git a/ethtool.8.in b/ethtool.8.in
index 12940e1b32aa..a87f31f90077 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -199,6 +199,7 @@ ethtool \- query or control network driver and hardware settings
.BN rx\-jumbo
.BN tx
.BN rx\-buf\-len
+.BN tx\-push
.HP
.B ethtool \-i|\-\-driver
.I devname
@@ -573,6 +574,9 @@ Changes the number of ring entries for the Tx ring.
.TP
.BI rx\-buf\-len \ N
Changes the size of a buffer in the Rx ring.
+.TP
+.BI tx\-push \ on|off
+Specifies whether TX push should be enabled.
.RE
.TP
.B \-i \-\-driver
diff --git a/ethtool.c b/ethtool.c
index 4f5c234304be..4d2a475051a7 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5733,6 +5733,7 @@ static const struct option args[] = {
" [ rx-jumbo N ]\n"
" [ tx N ]\n"
" [ rx-buf-len N]\n"
+ " [ tx-push on|off]\n"
},
{
.opts = "-k|--show-features|--show-offload",
diff --git a/netlink/rings.c b/netlink/rings.c
index 119178ea336b..a53eed5dbc60 100644
--- a/netlink/rings.c
+++ b/netlink/rings.c
@@ -47,6 +47,7 @@ int rings_reply_cb(const struct nlmsghdr *nlhdr, void *data)
show_u32(tb[ETHTOOL_A_RINGS_RX_JUMBO], "RX Jumbo:\t");
show_u32(tb[ETHTOOL_A_RINGS_TX], "TX:\t\t");
show_u32(tb[ETHTOOL_A_RINGS_RX_BUF_LEN], "RX Buf Len:\t\t");
+ show_bool("tx-push", "TX Push:\t%s\n", tb[ETHTOOL_A_RINGS_TX_PUSH]);
return MNL_CB_OK;
}
@@ -105,6 +106,12 @@ static const struct param_parser sring_params[] = {
.handler = nl_parse_direct_u32,
.min_argc = 1,
},
+ {
+ .arg = "tx-push",
+ .type = ETHTOOL_A_RINGS_TX_PUSH,
+ .handler = nl_parse_u8bool,
+ .min_argc = 0,
+ },
{}
};
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index d8b19cf98003..79fe0bf686f3 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -330,6 +330,7 @@ enum {
ETHTOOL_A_RINGS_RX_JUMBO, /* u32 */
ETHTOOL_A_RINGS_TX, /* u32 */
ETHTOOL_A_RINGS_RX_BUF_LEN, /* u32 */
+ ETHTOOL_A_RINGS_TX_PUSH = 13, /* u8 */
/* add new constants above here */
__ETHTOOL_A_RINGS_CNT,
--
2.33.0
^ permalink raw reply related
* [PATCH net-next] Revert "rtnetlink: return EINVAL when request cannot succeed"
From: Florent Fourcot @ 2022-04-19 12:51 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Brian Baboch, Florent Fourcot
In-Reply-To: <Yl6iFqPFrdvD1wam@zx2c4.com>
This reverts commit b6177d3240a4
ip-link command is testing kernel capability by sending a RTM_NEWLINK
request, without any argument. It accepts everything in reply, except
EOPNOTSUPP and EINVAL (functions iplink_have_newlink / accept_msg)
So we must keep compatiblity here, invalid empty message should not
return EINVAL
Signed-off-by: Florent Fourcot <florent.fourcot@wifirst.fr>
---
net/core/rtnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b943336908a7..73f2cbc440c9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3457,7 +3457,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
return rtnl_group_changelink(skb, net,
nla_get_u32(tb[IFLA_GROUP]),
ifm, extack, tb);
- return -EINVAL;
+ return -ENODEV;
}
if (tb[IFLA_MAP] || tb[IFLA_PROTINFO])
--
2.30.2
^ permalink raw reply related
* [PATCH net-next] nfp: support 802.1ad VLAN assingment to VF
From: Simon Horman @ 2022-04-19 12:44 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: netdev, oss-drivers
From: Baowen Zheng <baowen.zheng@corigine.com>
The NFP driver already supports assignment of 802.1Q VLANs to VFs
e.g.
# ip link set $DEV vf $VF_NUM vlan $VLAN_ID [proto 802.1Q]
This patch enhances the NFP driver to also allow assingment of
802.1ad VLANs to VFs.
e.g.
# ip link set $DEV vf $VF_NUM vlan $VLAN_ID proto 802.1ad
Signed-off-by: Bin Chen <bin.chen@corigine.com>
Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
Signed-off-by: Yinjun Zhang <yunjin.zhang@corigine.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../ethernet/netronome/nfp/nfp_net_sriov.c | 43 ++++++++++++++-----
.../ethernet/netronome/nfp/nfp_net_sriov.h | 3 ++
2 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.c b/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.c
index 3fdaaf8ed2ba..4627715a5e32 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.c
@@ -95,15 +95,17 @@ int nfp_app_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan, u8 qos,
__be16 vlan_proto)
{
struct nfp_app *app = nfp_app_from_netdev(netdev);
+ u16 update = NFP_NET_VF_CFG_MB_UPD_VLAN;
+ bool is_proto_sup = true;
unsigned int vf_offset;
- u16 vlan_tci;
+ u32 vlan_tag;
int err;
err = nfp_net_sriov_check(app, vf, NFP_NET_VF_CFG_MB_CAP_VLAN, "vlan");
if (err)
return err;
- if (vlan_proto != htons(ETH_P_8021Q))
+ if (!eth_type_vlan(vlan_proto))
return -EOPNOTSUPP;
if (vlan > 4095 || qos > 7) {
@@ -112,14 +114,32 @@ int nfp_app_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan, u8 qos,
return -EINVAL;
}
+ /* Check if fw supports or not */
+ err = nfp_net_sriov_check(app, vf, NFP_NET_VF_CFG_MB_CAP_VLAN_PROTO, "vlan_proto");
+ if (err)
+ is_proto_sup = false;
+
+ if (vlan_proto != htons(ETH_P_8021Q)) {
+ if (!is_proto_sup)
+ return -EOPNOTSUPP;
+ update |= NFP_NET_VF_CFG_MB_UPD_VLAN_PROTO;
+ }
+
/* Write VLAN tag to VF entry in VF config symbol */
- vlan_tci = FIELD_PREP(NFP_NET_VF_CFG_VLAN_VID, vlan) |
+ vlan_tag = FIELD_PREP(NFP_NET_VF_CFG_VLAN_VID, vlan) |
FIELD_PREP(NFP_NET_VF_CFG_VLAN_QOS, qos);
+
+ /* vlan_tag of 0 means that the configuration should be cleared and in
+ * such circumstances setting the TPID has no meaning when
+ * configuring firmware.
+ */
+ if (vlan_tag && is_proto_sup)
+ vlan_tag |= FIELD_PREP(NFP_NET_VF_CFG_VLAN_PROT, ntohs(vlan_proto));
+
vf_offset = NFP_NET_VF_CFG_MB_SZ + vf * NFP_NET_VF_CFG_SZ;
- writew(vlan_tci, app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_VLAN);
+ writel(vlan_tag, app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_VLAN);
- return nfp_net_sriov_update(app, vf, NFP_NET_VF_CFG_MB_UPD_VLAN,
- "vlan");
+ return nfp_net_sriov_update(app, vf, update, "vlan");
}
int nfp_app_set_vf_spoofchk(struct net_device *netdev, int vf, bool enable)
@@ -209,7 +229,7 @@ int nfp_app_get_vf_config(struct net_device *netdev, int vf,
{
struct nfp_app *app = nfp_app_from_netdev(netdev);
unsigned int vf_offset;
- u16 vlan_tci;
+ u32 vlan_tag;
u32 mac_hi;
u16 mac_lo;
u8 flags;
@@ -225,7 +245,7 @@ int nfp_app_get_vf_config(struct net_device *netdev, int vf,
mac_lo = readw(app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_MAC_LO);
flags = readb(app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_CTRL);
- vlan_tci = readw(app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_VLAN);
+ vlan_tag = readl(app->pf->vfcfg_tbl2 + vf_offset + NFP_NET_VF_CFG_VLAN);
memset(ivi, 0, sizeof(*ivi));
ivi->vf = vf;
@@ -233,9 +253,10 @@ int nfp_app_get_vf_config(struct net_device *netdev, int vf,
put_unaligned_be32(mac_hi, &ivi->mac[0]);
put_unaligned_be16(mac_lo, &ivi->mac[4]);
- ivi->vlan = FIELD_GET(NFP_NET_VF_CFG_VLAN_VID, vlan_tci);
- ivi->qos = FIELD_GET(NFP_NET_VF_CFG_VLAN_QOS, vlan_tci);
-
+ ivi->vlan = FIELD_GET(NFP_NET_VF_CFG_VLAN_VID, vlan_tag);
+ ivi->qos = FIELD_GET(NFP_NET_VF_CFG_VLAN_QOS, vlan_tag);
+ if (!nfp_net_sriov_check(app, vf, NFP_NET_VF_CFG_MB_CAP_VLAN_PROTO, "vlan_proto"))
+ ivi->vlan_proto = htons(FIELD_GET(NFP_NET_VF_CFG_VLAN_PROT, vlan_tag));
ivi->spoofchk = FIELD_GET(NFP_NET_VF_CFG_CTRL_SPOOF, flags);
ivi->trusted = FIELD_GET(NFP_NET_VF_CFG_CTRL_TRUST, flags);
ivi->linkstate = FIELD_GET(NFP_NET_VF_CFG_CTRL_LINK_STATE, flags);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.h b/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.h
index 786be58a907e..7b72cc083476 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_sriov.h
@@ -19,6 +19,7 @@
#define NFP_NET_VF_CFG_MB_CAP_SPOOF (0x1 << 2)
#define NFP_NET_VF_CFG_MB_CAP_LINK_STATE (0x1 << 3)
#define NFP_NET_VF_CFG_MB_CAP_TRUST (0x1 << 4)
+#define NFP_NET_VF_CFG_MB_CAP_VLAN_PROTO (0x1 << 5)
#define NFP_NET_VF_CFG_MB_RET 0x2
#define NFP_NET_VF_CFG_MB_UPD 0x4
#define NFP_NET_VF_CFG_MB_UPD_MAC (0x1 << 0)
@@ -26,6 +27,7 @@
#define NFP_NET_VF_CFG_MB_UPD_SPOOF (0x1 << 2)
#define NFP_NET_VF_CFG_MB_UPD_LINK_STATE (0x1 << 3)
#define NFP_NET_VF_CFG_MB_UPD_TRUST (0x1 << 4)
+#define NFP_NET_VF_CFG_MB_UPD_VLAN_PROTO (0x1 << 5)
#define NFP_NET_VF_CFG_MB_VF_NUM 0x7
/* VF config entry
@@ -43,6 +45,7 @@
#define NFP_NET_VF_CFG_LS_MODE_ENABLE 1
#define NFP_NET_VF_CFG_LS_MODE_DISABLE 2
#define NFP_NET_VF_CFG_VLAN 0x8
+#define NFP_NET_VF_CFG_VLAN_PROT 0xffff0000
#define NFP_NET_VF_CFG_VLAN_QOS 0xe000
#define NFP_NET_VF_CFG_VLAN_VID 0x0fff
--
2.30.2
^ permalink raw reply related
* Re: [RFC PATCH net-next 2/2] net: phy: micrel: Implement set/get_adj_latency for lan8814
From: Andrew Lunn @ 2022-04-19 12:42 UTC (permalink / raw)
To: Horatiu Vultur
Cc: netdev, linux-kernel, hkallweit1, linux, davem, kuba, pabeni,
UNGLinuxDriver, richardcochran
In-Reply-To: <20220419083704.48573-3-horatiu.vultur@microchip.com>
On Tue, Apr 19, 2022 at 10:37:04AM +0200, Horatiu Vultur wrote:
> The lan8814 driver supports adjustments of the latency in the silicon
> based on the speed and direction, therefore implement set/get_adj_latency
> to adjust the HW.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> drivers/net/phy/micrel.c | 87 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 96840695debd..099d1ecd6dad 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -120,6 +120,15 @@
> #define PTP_TIMESTAMP_EN_PDREQ_ BIT(2)
> #define PTP_TIMESTAMP_EN_PDRES_ BIT(3)
>
> +#define PTP_RX_LATENCY_1000 0x0224
> +#define PTP_TX_LATENCY_1000 0x0225
> +
> +#define PTP_RX_LATENCY_100 0x0222
> +#define PTP_TX_LATENCY_100 0x0223
> +
> +#define PTP_RX_LATENCY_10 0x0220
> +#define PTP_TX_LATENCY_10 0x0221
> +
> #define PTP_TX_PARSE_L2_ADDR_EN 0x0284
> #define PTP_RX_PARSE_L2_ADDR_EN 0x0244
>
> @@ -208,6 +217,16 @@
> #define PTP_TSU_INT_STS_PTP_RX_TS_OVRFL_INT_ BIT(1)
> #define PTP_TSU_INT_STS_PTP_RX_TS_EN_ BIT(0)
>
> +/* Represents the reset value of the latency registers,
> + * The values are express in ns
> + */
> +#define LAN8814_RX_10_LATENCY 8874
> +#define LAN8814_TX_10_LATENCY 11850
> +#define LAN8814_RX_100_LATENCY 2346
> +#define LAN8814_TX_100_LATENCY 705
> +#define LAN8814_RX_1000_LATENCY 429
> +#define LAN8814_TX_1000_LATENCY 201
> +
> /* PHY Control 1 */
> #define MII_KSZPHY_CTRL_1 0x1e
> #define KSZ8081_CTRL1_MDIX_STAT BIT(4)
> @@ -2657,6 +2676,72 @@ static int lan8804_config_init(struct phy_device *phydev)
> return 0;
> }
>
> +static int lan8814_set_adj_latency(struct phy_device *phydev,
> + enum ethtool_link_mode_bit_indices link_mode,
> + s32 rx, s32 tx)
> +{
> + switch (link_mode) {
> + case ETHTOOL_LINK_MODE_10baseT_Half_BIT:
> + case ETHTOOL_LINK_MODE_10baseT_Full_BIT:
> + rx += LAN8814_RX_10_LATENCY;
> + tx += LAN8814_TX_10_LATENCY;
> + lanphy_write_page_reg(phydev, 5, PTP_RX_LATENCY_10, rx);
> + lanphy_write_page_reg(phydev, 5, PTP_TX_LATENCY_10, tx);
It is not ideal that the user sees an entry for both link modes X and
X+1 in your file, and that writing to link mode X magically also
changes X+1.
I'm not sure there is anything you can do about this in a generic
implementation, so you at least need to document it in sysfs.
What about range checks? I can pass 32764 as an rx delay, which when
added to PTP_RX_LATENCY_10=0x0220 is going to wrap around and be
negative.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 1/2] net: phy: Add phy latency adjustment support in phy framework.
From: Andrew Lunn @ 2022-04-19 12:32 UTC (permalink / raw)
To: Horatiu Vultur
Cc: netdev, linux-kernel, hkallweit1, linux, davem, kuba, pabeni,
UNGLinuxDriver, richardcochran
In-Reply-To: <20220419083704.48573-2-horatiu.vultur@microchip.com>
On Tue, Apr 19, 2022 at 10:37:03AM +0200, Horatiu Vultur wrote:
> Add adjustment support for latency for the phy using sysfs.
> This is used to adjust the latency of the phy based on link mode
> and direction.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> .../ABI/testing/sysfs-class-net-phydev | 10 ++++
> drivers/net/phy/phy_device.c | 58 +++++++++++++++++++
> include/linux/phy.h | 9 +++
> 3 files changed, 77 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-net-phydev b/Documentation/ABI/testing/sysfs-class-net-phydev
> index ac722dd5e694..a99bbfeddb6f 100644
> --- a/Documentation/ABI/testing/sysfs-class-net-phydev
> +++ b/Documentation/ABI/testing/sysfs-class-net-phydev
> @@ -63,3 +63,13 @@ Description:
> only used internally by the kernel and their placement are
> not meant to be stable across kernel versions. This is intended
> for facilitating the debugging of PHY drivers.
> +
> +What: /sys/class/mdio_bus/<bus>/<device>/adjust_latency
> +Date: April 2022
> +KernelVersion: 5.19
> +Contact: netdev@vger.kernel.org
> +Description:
> + This file adjusts the latency in the PHY. To set value,
> + write three integers into the file: interface mode, RX latency,
> + TX latency. When the file is read, it returns the supported
> + interface modes and the latency values.
https://lwn.net/Articles/378884/
The key design goal relating to attribute files is the
stipulation - almost a mantra - of "one file, one value" or
sometimes "one item per file". The idea here is that each
attribute file should contain precisely one value. If multiple
values are needed, then multiple files should be used.
You also need to specify units in the documentation.
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 8406ac739def..80bf04ca0e02 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -529,6 +529,48 @@ static ssize_t phy_dev_flags_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(phy_dev_flags);
>
> +static ssize_t adjust_latency_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct phy_device *phydev = to_phy_device(dev);
> + ssize_t count = 0;
> + int err, i;
> + s32 rx, tx;
> +
> + for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; ++i) {
> + err = phydev->drv->get_adj_latency(phydev, i, &rx, &tx);
> + if (err == -EINVAL)
-EOPNOTSUPP seems more likley than EINVAL.
> + continue;
> +
> + count += sprintf(&buf[count], "%d rx: %d, tx: %d\n", i, rx, tx);
Link modes as a number? Can you tell me off the top of you head what
link mode 42 is?
Also, if phydev->drv->get_adj_latency() returns any other error, it is
likely rx and tx contain rubbish, yet you still add them to the
output.
> @@ -932,6 +932,15 @@ struct phy_driver {
> int (*get_sqi)(struct phy_device *dev);
> /** @get_sqi_max: Get the maximum signal quality indication */
> int (*get_sqi_max)(struct phy_device *dev);
> + /** @set_adj_latency: Set the latency values of the PHY */
> + int (*set_adj_latency)(struct phy_device *dev,
> + enum ethtool_link_mode_bit_indices link_mode,
> + s32 rx, s32 tx);
> + /** @get_latency: Get the latency values of the PHY */
> + int (*get_adj_latency)(struct phy_device *dev,
> + enum ethtool_link_mode_bit_indices link_mode,
> + s32 *rx, s32 *tx);
You need to clearly document the return value here, that -EINVAL (or
maybe -EOPNOTUSPP) has special meaning. I would also document the
units, and what is supposed to happen if the stamper already has a
hard coded offset.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 0/2] net: phy: Extend sysfs to adjust PHY latency.
From: Andrew Lunn @ 2022-04-19 12:17 UTC (permalink / raw)
To: Horatiu Vultur
Cc: netdev, linux-kernel, hkallweit1, linux, davem, kuba, pabeni,
UNGLinuxDriver, richardcochran
In-Reply-To: <20220419083704.48573-1-horatiu.vultur@microchip.com>
On Tue, Apr 19, 2022 at 10:37:02AM +0200, Horatiu Vultur wrote:
> The previous try of setting the PHY latency was here[1]. But this approach
> could not work for multiple reasons:
> - the interface was not generic enough so it would be hard to be extended
> in the future
> - if there were multiple time stamper in the system then it was not clear
> to which one should adjust these values.
>
> So the next try is to extend sysfs and configure exactly the desired PHY.
What about timestampers which are not PHYs? Ideally you want one
interface which will work for any sort of stamper, be it MAC, PHY, or
a bump in the wire between the MAC and the PHY.
Andrew
^ permalink raw reply
* Re: [PATCH] net: phy: fix error check return value of phy_read()
From: Andrew Lunn @ 2022-04-19 12:07 UTC (permalink / raw)
To: cgel.zte
Cc: f.fainelli, bcm-kernel-feedback-list, hkallweit1, linux, davem,
kuba, pabeni, netdev, linux-kernel, Lv Ruyi, Zeal Robot
In-Reply-To: <20220419014439.2561835-1-lv.ruyi@zte.com.cn>
On Tue, Apr 19, 2022 at 01:44:39AM +0000, cgel.zte@gmail.com wrote:
> From: Lv Ruyi <lv.ruyi@zte.com.cn>
>
> phy_read() returns a negative number if there's an error, but the
> error-checking code in the bcm87xx driver's config_intr function
> triggers if phy_read() returns non-zero. Correct that.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn>
> ---
> drivers/net/phy/bcm87xx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> index 313563482690..e62b53718010 100644
> --- a/drivers/net/phy/bcm87xx.c
> +++ b/drivers/net/phy/bcm87xx.c
> @@ -146,7 +146,7 @@ static int bcm87xx_config_intr(struct phy_device *phydev)
>
> if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> err = phy_read(phydev, BCM87XX_LASI_STATUS);
> - if (err)
> + if (err < 0)
> return err;
This should probably have a Fixes: tag, and be for net, not next-next.
Please read the netdev FAQ about the trees, and submittinng fixes for
netdev.
Andrew
^ permalink raw reply
* Re: [PATCH] net: ethernet: mtk_eth_soc: fix error check return value of debugfs_create_dir()
From: Andrew Lunn @ 2022-04-19 12:03 UTC (permalink / raw)
To: cgel.zte
Cc: f.fainelli, bcm-kernel-feedback-list, hkallweit1, linux, davem,
kuba, pabeni, netdev, linux-kernel, Lv Ruyi, Zeal Robot
In-Reply-To: <20220419014056.2561750-1-lv.ruyi@zte.com.cn>
On Tue, Apr 19, 2022 at 01:40:56AM +0000, cgel.zte@gmail.com wrote:
> From: Lv Ruyi <lv.ruyi@zte.com.cn>
>
> If an error occurs, debugfs_create_file() will return ERR_PTR(-ERROR),
> so use IS_ERR() to check it.
>
> Fixes: 804775dfc288 ("net: ethernet: mtk_eth_soc: add support for Wireless Ethernet Dispatch (WED)")
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn>
> ---
> drivers/net/ethernet/mediatek/mtk_wed_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_wed_debugfs.c b/drivers/net/ethernet/mediatek/mtk_wed_debugfs.c
> index a81d3fd1a439..0c284c18a8d7 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed_debugfs.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed_debugfs.c
> @@ -165,7 +165,7 @@ void mtk_wed_hw_add_debugfs(struct mtk_wed_hw *hw)
>
> snprintf(hw->dirname, sizeof(hw->dirname), "wed%d", hw->index);
> dir = debugfs_create_dir(hw->dirname, NULL);
> - if (!dir)
> + if (IS_ERR(dir))
> return;
You should not check the return value from any debugfs calls.
If Zeal bot is saying you should, Zeal is broken/does not understand
debugfs.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 00/17] Introduce line card support for modular switch
From: Jiri Pirko @ 2022-04-19 11:55 UTC (permalink / raw)
To: David Ahern
Cc: Ido Schimmel, netdev, davem, kuba, pabeni, jiri, vadimp, petrm,
andrew, mlxsw
In-Reply-To: <4d86acf1-d449-92d7-f8c7-bd0edc9e5107@gmail.com>
Mon, Apr 18, 2022 at 04:31:30PM CEST, dsahern@gmail.com wrote:
>On 4/18/22 12:42 AM, Ido Schimmel wrote:
>> Jiri says:
>>
>> This patchset introduces support for modular switch systems and also
>> introduces mlxsw support for NVIDIA Mellanox SN4800 modular switch.
>> It contains 8 slots to accommodate line cards - replaceable PHY modules
>> which may contain gearboxes.
>> Currently supported line card:
>> 16X 100GbE (QSFP28)
>> Other line cards that are going to be supported:
>> 8X 200GbE (QSFP56)
>> 4X 400GbE (QSFP-DD)
>> There may be other types of line cards added in the future.
>>
>> To be consistent with the port split configuration (splitter cabels),
>> the line card entities are treated in the similar way. The nature of
>> a line card is not "a pluggable device", but "a pluggable PHY module".
>>
>> A concept of "provisioning" is introduced. The user may "provision"
>> certain slot with a line card type. Driver then creates all instances
>> (devlink ports, netdevices, etc) related to this line card type. It does
>> not matter if the line card is plugged-in at the time. User is able to
>> configure netdevices, devlink ports, setup port splitters, etc. From the
>> perspective of the switch ASIC, all is present and can be configured.
>>
>> The carrier of netdevices stays down if the line card is not plugged-in.
>> Once the line card is inserted and activated, the carrier of
>> the related netdevices is then reflecting the physical line state,
>> same as for an ordinary fixed port.
>>
>> Once user does not want to use the line card related instances
>> anymore, he can "unprovision" the slot. Driver then removes the
>> instances.
>>
>> Patches 1-4 are extending devlink driver API and UAPI in order to
>> register, show, dump, provision and activate the line card.
>> Patches 5-17 are implementing the introduced API in mlxsw.
>> The last patch adds a selftest for mlxsw line cards.
>>
>> Example:
>> $ devlink port # No ports are listed
>> $ devlink lc
>> pci/0000:01:00.0:
>> lc 1 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 2 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 3 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 4 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 5 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 6 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 7 state unprovisioned
>> supported_types:
>> 16x100G
>> lc 8 state unprovisioned
>> supported_types:
>> 16x100G
>>
>> Note that driver exposes list supported line card types. Currently
>> there is only one: "16x100G".
>>
>> To provision the slot #8:
>>
>> $ devlink lc set pci/0000:01:00.0 lc 8 type 16x100G
>> $ devlink lc show pci/0000:01:00.0 lc 8
>> pci/0000:01:00.0:
>> lc 8 state active type 16x100G
>> supported_types:
>> 16x100G
>> $ devlink port
>> pci/0000:01:00.0/0: type notset flavour cpu port 0 splittable false
>> pci/0000:01:00.0/53: type eth netdev enp1s0nl8p1 flavour physical lc 8 port 1 splittable true lanes 4
>> pci/0000:01:00.0/54: type eth netdev enp1s0nl8p2 flavour physical lc 8 port 2 splittable true lanes 4
>> pci/0000:01:00.0/55: type eth netdev enp1s0nl8p3 flavour physical lc 8 port 3 splittable true lanes 4
>> pci/0000:01:00.0/56: type eth netdev enp1s0nl8p4 flavour physical lc 8 port 4 splittable true lanes 4
>> pci/0000:01:00.0/57: type eth netdev enp1s0nl8p5 flavour physical lc 8 port 5 splittable true lanes 4
>> pci/0000:01:00.0/58: type eth netdev enp1s0nl8p6 flavour physical lc 8 port 6 splittable true lanes 4
>> pci/0000:01:00.0/59: type eth netdev enp1s0nl8p7 flavour physical lc 8 port 7 splittable true lanes 4
>> pci/0000:01:00.0/60: type eth netdev enp1s0nl8p8 flavour physical lc 8 port 8 splittable true lanes 4
>> pci/0000:01:00.0/61: type eth netdev enp1s0nl8p9 flavour physical lc 8 port 9 splittable true lanes 4
>> pci/0000:01:00.0/62: type eth netdev enp1s0nl8p10 flavour physical lc 8 port 10 splittable true lanes 4
>> pci/0000:01:00.0/63: type eth netdev enp1s0nl8p11 flavour physical lc 8 port 11 splittable true lanes 4
>> pci/0000:01:00.0/64: type eth netdev enp1s0nl8p12 flavour physical lc 8 port 12 splittable true lanes 4
>> pci/0000:01:00.0/125: type eth netdev enp1s0nl8p13 flavour physical lc 8 port 13 splittable true lanes 4
>> pci/0000:01:00.0/126: type eth netdev enp1s0nl8p14 flavour physical lc 8 port 14 splittable true lanes 4
>> pci/0000:01:00.0/127: type eth netdev enp1s0nl8p15 flavour physical lc 8 port 15 splittable true lanes 4
>> pci/0000:01:00.0/128: type eth netdev enp1s0nl8p16 flavour physical lc 8 port 16 splittable true lanes 4
>>
>> To uprovision the slot #8:
>>
>> $ devlink lc set pci/0000:01:00.0 lc 8 notype
>>
>
>are there any changes from the last RFC?
>
>https://lore.kernel.org/netdev/20210122094648.1631078-1-jiri@resnulli.us/
Yes, many of them, I din't track them. Mainly, the RFC was backed by
netdevsim implementation, this is mlxsw with actual HW underneath.
^ permalink raw reply
* Re: [PATCH v5 net-next 0/4] rtnetlink: improve ALT_IFNAME config and fix dangerous GROUP usage
From: Jason A. Donenfeld @ 2022-04-19 11:50 UTC (permalink / raw)
To: Florent Fourcot; +Cc: netdev, cong.wang, edumazet
In-Reply-To: <20220415165330.10497-1-florent.fourcot@wifirst.fr>
Hi Florent,
On Fri, Apr 15, 2022 at 06:53:26PM +0200, Florent Fourcot wrote:
> First commit forbids dangerous calls when both IFNAME and GROUP are
> given, since it can introduce unexpected behaviour when IFNAME does not
> match any interface.
>
> Second patch achieves primary goal of this patchset to fix/improve
> IFLA_ALT_IFNAME attribute, since previous code was never working for
> newlink/setlink. ip-link command is probably getting interface index
> before, and was not using this feature.
>
> Last two patches are improving error code on corner cases.
This was just merged to net-next and appears to have broken the
wireguard test suite over on https://build.wireguard.com/
[+] Launching tests...
[ 0.796066] init.sh (28) used greatest stack depth: 29152 bytes left
[ 0.803809] ip (29) used greatest stack depth: 28544 bytes left
[+] ip netns add wg-test-27-0
[+] ip netns add wg-test-27-1
[ 0.842841] ip (32) used greatest stack depth: 27512 bytes left
[+] ip netns add wg-test-27-2
[+] NS0: ip link set up dev lo
[+] NS0: ip link add dev wg0 type wireguard
[ 0.896074] ip (35) used greatest stack depth: 27152 bytes left
Command "add" is unknown, try "ip link help".
[+] NS0: ip link del dev wg0
[+] NS0: ip link del dev wg1
[+] NS1: ip link del dev wg0
[+] NS1: ip link del dev wg1
[+] NS2: ip link del dev wg0
[+] NS2: ip link del dev wg1
[+] ip netns del wg-test-27-1
[+] ip netns del wg-test-27-2
[+] ip netns del wg-test-27-0
[-] Tests failed with exit code 255! ☹
So apparently something goes wrong with "ip link add dev wg0 type
wireguard". Not quite sure what yet, though. You can try it for yourself
with:
make -C tools/testing/selftests/wireguard/qemu -j$(nproc)
Jason
^ permalink raw reply
* Re: [PATCH v5 net-next 0/4] rtnetlink: improve ALT_IFNAME config and fix dangerous GROUP usage
From: patchwork-bot+netdevbpf @ 2022-04-19 11:50 UTC (permalink / raw)
To: Florent Fourcot; +Cc: netdev, cong.wang, edumazet
In-Reply-To: <20220415165330.10497-1-florent.fourcot@wifirst.fr>
Hello:
This series was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 15 Apr 2022 18:53:26 +0200 you wrote:
> First commit forbids dangerous calls when both IFNAME and GROUP are
> given, since it can introduce unexpected behaviour when IFNAME does not
> match any interface.
>
> Second patch achieves primary goal of this patchset to fix/improve
> IFLA_ALT_IFNAME attribute, since previous code was never working for
> newlink/setlink. ip-link command is probably getting interface index
> before, and was not using this feature.
>
> [...]
Here is the summary with links:
- [v5,net-next,1/4] rtnetlink: return ENODEV when ifname does not exist and group is given
https://git.kernel.org/netdev/net-next/c/ef2a7c9065ce
- [v5,net-next,2/4] rtnetlink: enable alt_ifname for setlink/newlink
https://git.kernel.org/netdev/net-next/c/5ea08b5286f6
- [v5,net-next,3/4] rtnetlink: return ENODEV when IFLA_ALT_IFNAME is used in dellink
https://git.kernel.org/netdev/net-next/c/dee04163e9f2
- [v5,net-next,4/4] rtnetlink: return EINVAL when request cannot succeed
https://git.kernel.org/netdev/net-next/c/b6177d3240a4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH] driver: usb: nullify dangling pointer in cdc_ncm_free
From: Oliver Neukum @ 2022-04-19 11:47 UTC (permalink / raw)
To: Andy Shevchenko, Johan Hovold, Oleksij Rempel
Cc: Dongliang Mu, Oliver Neukum, David S. Miller, Jakub Kicinski,
Paolo Abeni, Dongliang Mu, syzbot+eabbf2aaa999cc507108, USB,
netdev, Linux Kernel Mailing List
In-Reply-To: <CAHp75VeTqmdLhavZ+VbBYSFMDHr0FG4iKFGdbzE-wo5MCNikAA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]
On 14.04.22 17:01, Andy Shevchenko wrote:
>
> Good question. Isn't it the commit 2c9d6c2b871d ("usbnet: run unbind()
> before unregister_netdev()") which changed the ordering of the
> interface shutdown and basically makes this race happen? I don't see
> how we can guarantee that IOCTL won't be called until we quiescence
> the network device — my understanding that on device surprise removal
True. The best we could do is introduce a mutex for ioctl() and
disconnect(). That seems the least preferable solution to me.
> we have to first shutdown what it created and then unbind the device.
> If I understand the original issue correctly then the problem is in
> usbnet->unbind and it should actually be split to two hooks, otherwise
> it seems every possible IOCTL callback must have some kind of
> reference counting and keep an eye on the surprise removal.
>
> Johan, can you correct me if my understanding is wrong?
>
It seems to me that fundamentally the order of actions to handle
a hotunplug must mirror the order in a hotplug. We can add more hooks
if that turns out to be necessary for some drivers, but the basic
reverse mirrored order must be supported and I very much favor
restoring it as default.
So I am afraid I have to ask again, whether anybody sees a fundamental
issue with the attached patch, as opposed to it not being an elegant
solution?
It looks to me that we are in a fundamental disagreement on the correct
order in this question and there is no productive way forward other than
offering both ways.
Regards
Oliver
[-- Attachment #2: 0001-usbnet-split-unbind-callback.patch --]
[-- Type: text/x-patch, Size: 4357 bytes --]
From 2e07ccbd1769889963d129ec474909bdcaa4c64a Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Thu, 10 Mar 2022 13:18:38 +0100
Subject: [PATCH] usbnet: split unbind callback
Some devices need to be informed of a disconnect before
the generic layer is informed, others need their notification
later to avoid race conditions. Hence we provide two callbacks.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/net/usb/asix_devices.c | 8 ++++----
drivers/net/usb/smsc95xx.c | 4 ++--
drivers/net/usb/usbnet.c | 7 +++++--
include/linux/usb/usbnet.h | 3 +++
4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 6ea44e53713a..e6cfa9a39a87 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -808,7 +808,7 @@ static int ax88772_stop(struct usbnet *dev)
return 0;
}
-static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
+static void ax88772_disable(struct usbnet *dev, struct usb_interface *intf)
{
struct asix_common_private *priv = dev->driver_priv;
@@ -1214,7 +1214,7 @@ static const struct driver_info hawking_uf200_info = {
static const struct driver_info ax88772_info = {
.description = "ASIX AX88772 USB 2.0 Ethernet",
.bind = ax88772_bind,
- .unbind = ax88772_unbind,
+ .unbind = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
@@ -1226,7 +1226,7 @@ static const struct driver_info ax88772_info = {
static const struct driver_info ax88772b_info = {
.description = "ASIX AX88772B USB 2.0 Ethernet",
.bind = ax88772_bind,
- .unbind = ax88772_unbind,
+ .unbind = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
@@ -1262,7 +1262,7 @@ static const struct driver_info ax88178_info = {
static const struct driver_info hg20f9_info = {
.description = "HG20F9 USB 2.0 Ethernet",
.bind = ax88772_bind,
- .unbind = ax88772_unbind,
+ .unbind = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 5567220e9d16..62db57021f5f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1211,7 +1211,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
return ret;
}
-static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
+static void smsc95xx_disable(struct usbnet *dev, struct usb_interface *intf)
{
struct smsc95xx_priv *pdata = dev->driver_priv;
@@ -1985,7 +1985,7 @@ static int smsc95xx_manage_power(struct usbnet *dev, int on)
static const struct driver_info smsc95xx_info = {
.description = "smsc95xx USB 2.0 Ethernet",
.bind = smsc95xx_bind,
- .unbind = smsc95xx_unbind,
+ .unbind = smsc95xx_disable,
.link_reset = smsc95xx_link_reset,
.reset = smsc95xx_reset,
.check_connect = smsc95xx_start_phy,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index b1f93810a6f3..5249a7d7efa5 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1641,8 +1641,8 @@ void usbnet_disconnect (struct usb_interface *intf)
xdev->bus->bus_name, xdev->devpath,
dev->driver_info->description);
- if (dev->driver_info->unbind)
- dev->driver_info->unbind(dev, intf);
+ if (dev->driver_info->disable)
+ dev->driver_info->disable(dev, intf);
net = dev->net;
unregister_netdev (net);
@@ -1651,6 +1651,9 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_scuttle_anchored_urbs(&dev->deferred);
+ if (dev->driver_info->unbind)
+ dev->driver_info->unbind (dev, intf);
+
usb_kill_urb(dev->interrupt);
usb_free_urb(dev->interrupt);
kfree(dev->padding_pkt);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 8336e86ce606..4d2407f1ae93 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -129,6 +129,9 @@ struct driver_info {
/* cleanup device ... can sleep, but can't fail */
void (*unbind)(struct usbnet *, struct usb_interface *);
+ /* disable device ... can sleep, but can't fail */
+ void (*disable)(struct usbnet *, struct usb_interface *);
+
/* reset device ... can sleep */
int (*reset)(struct usbnet *);
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next] samples/bpf: reduce the sampling interval in xdp1_user
From: Zhengchao Shao @ 2022-04-19 11:47 UTC (permalink / raw)
To: bpf, netdev, linux-kernel, ast, daniel, davem, kuba, hawk,
john.fastabend, andrii, kafai, songliubraving, yhs, kpsingh
Cc: weiyongjun1, shaozhengchao, yuehaibing
If interval is 2, and sum - prev[key] = 1, the result = 0. This will
mislead the tester that the port has no traffic right now. So reduce the
sampling interval to 1.
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
samples/bpf/xdp1_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 631f0cabe139..bacebb4b602f 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -161,7 +161,7 @@ int main(int argc, char **argv)
}
prog_id = info.id;
- poll_stats(map_fd, 2);
+ poll_stats(map_fd, 1);
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v8 03/13] arm64: dts: freescale: Add the imx8dxl connectivity subsys dtsi
From: Krzysztof Kozlowski @ 2022-04-19 11:43 UTC (permalink / raw)
To: Abel Vesa, Rob Herring, Ulf Hansson, Shawn Guo, Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, Linux Kernel Mailing List, linux-mmc, netdev,
linux-arm-kernel, Jacky Bai
In-Reply-To: <20220419113516.1827863-4-abel.vesa@nxp.com>
On 19/04/2022 13:35, Abel Vesa wrote:
(...)
> +
> + usbmisc2: usbmisc@5b0e0200 {
Generic node names, so maybe "usb"?
> + #index-cells = <1>;
> + compatible = "fsl,imx8dxl-usbmisc", "fsl,imx7ulp-usbmisc";
> + reg = <0x5b0e0200 0x200>;
> + };
> +
> + usbphy2: usbphy@5b110000 {
Generic node names, so "phy".
> + compatible = "fsl,imx8dxl-usbphy", "fsl,imx7ulp-usbphy";
> + reg = <0x5b110000 0x1000>;
> + clocks = <&usb2_2_lpcg IMX_LPCG_CLK_7>;
> + status = "disabled";
> + };
> +
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v8 01/13] arm64: dts: freescale: Add the top level dtsi support for imx8dxl
From: Krzysztof Kozlowski @ 2022-04-19 11:41 UTC (permalink / raw)
To: Abel Vesa, Rob Herring, Ulf Hansson, Shawn Guo, Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, Linux Kernel Mailing List, linux-mmc, netdev,
linux-arm-kernel, Jacky Bai
In-Reply-To: <20220419113516.1827863-2-abel.vesa@nxp.com>
On 19/04/2022 13:35, Abel Vesa wrote:
> From: Jacky Bai <ping.bai@nxp.com>
>
> The i.MX8DXL is a device targeting the automotive and industrial
> market segments. The flexibility of the architecture allows for
> use in a wide variety of general embedded applications. The chip
> is designed to achieve both high performance and low power consumption.
> The chip relies on the power efficient dual (2x) Cortex-A35 cluster.
>
> Add the reserved memory node property for dsp reserved memory,
> the wakeup-irq property for SCU node, the rpmsg and the cm4 rproc
> support.
>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
(...)
> + thermal_zones: thermal-zones {
> + cpu-thermal0 {
Same concerns as you DTS.
> + polling-delay-passive = <250>;
> + polling-delay = <2000>;
> + thermal-sensors = <&tsens IMX_SC_R_SYSTEM>;
> +
> + trips {
> + cpu_alert0: trip0 {
> + temperature = <107000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + cpu_crit0: trip1 {
> + temperature = <127000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_alert0>;
> + cooling-device =
> + <&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A35_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> + };
> + };
> + };
> + };
> +
> + clk_dummy: clock-dummy {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <0>;
> + clock-output-names = "clk_dummy";
> + };
> +
> + xtal32k: clock-xtal32k {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + clock-output-names = "xtal_32KHz";
> + };
> +
> + xtal24m: clock-xtal24m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal_24MHz";
> + };
> +
> + /* sorted in register address */
> + #include "imx8-ss-adma.dtsi"
> + #include "imx8-ss-conn.dtsi"
> + #include "imx8-ss-ddr.dtsi"
> + #include "imx8-ss-lsio.dtsi"
> +};
> +
> +#include "imx8dxl-ss-adma.dtsi"
There is no such file. Your changes are not bisectable.
> +#include "imx8dxl-ss-conn.dtsi"
> +#include "imx8dxl-ss-lsio.dtsi"
> +#include "imx8dxl-ss-ddr.dtsi"
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 4.14 1/1] can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in error path
From: Dragos-Marian Panait @ 2022-04-19 11:38 UTC (permalink / raw)
To: stable
Cc: dragos.panait, wg, mkl, davem, paskripkin, gregkh, hbh25y,
linux-can, netdev, linux-kernel
In-Reply-To: <20220419113834.3116927-1-dragos.panait@windriver.com>
From: Hangyu Hua <hbh25y@gmail.com>
commit 3d3925ff6433f98992685a9679613a2cc97f3ce2 upstream.
There is no need to call dev_kfree_skb() when usb_submit_urb() fails
because can_put_echo_skb() deletes original skb and
can_free_echo_skb() deletes the cloned skb.
Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices")
Link: https://lore.kernel.org/all/20220311080614.45229-1-hbh25y@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
[DP: adjusted params of can_free_echo_skb() for 4.14 stable]
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
index df99354ec12a..232f45f722f0 100644
--- a/drivers/net/can/usb/usb_8dev.c
+++ b/drivers/net/can/usb/usb_8dev.c
@@ -681,9 +681,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
atomic_inc(&priv->active_tx_urbs);
err = usb_submit_urb(urb, GFP_ATOMIC);
- if (unlikely(err))
- goto failed;
- else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
+ if (unlikely(err)) {
+ can_free_echo_skb(netdev, context->echo_index);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
+
+ atomic_dec(&priv->active_tx_urbs);
+
+ if (err == -ENODEV)
+ netif_device_detach(netdev);
+ else
+ netdev_warn(netdev, "failed tx_urb %d\n", err);
+ stats->tx_dropped++;
+ } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
/* Slow down tx path */
netif_stop_queue(netdev);
@@ -702,19 +713,6 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
return NETDEV_TX_BUSY;
-failed:
- can_free_echo_skb(netdev, context->echo_index);
-
- usb_unanchor_urb(urb);
- usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
-
- atomic_dec(&priv->active_tx_urbs);
-
- if (err == -ENODEV)
- netif_device_detach(netdev);
- else
- netdev_warn(netdev, "failed tx_urb %d\n", err);
-
nomembuf:
usb_free_urb(urb);
--
2.17.1
^ permalink raw reply related
* [PATCH 4.14 0/1] can: usb_8dev: backport fix for CVE-2022-28388
From: Dragos-Marian Panait @ 2022-04-19 11:38 UTC (permalink / raw)
To: stable
Cc: dragos.panait, wg, mkl, davem, paskripkin, gregkh, hbh25y,
linux-can, netdev, linux-kernel
The following commit is needed to fix CVE-2022-28388:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3d3925ff6433f98992685a9679613a2cc97f3ce2
Hangyu Hua (1):
can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in
error path
drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
base-commit: 74766a973637a02c32c04c1c6496e114e4855239
--
2.17.1
^ permalink raw reply
* [PATCH 5.4 0/1] can: usb_8dev: backport fix for CVE-2022-28388
From: Dragos-Marian Panait @ 2022-04-19 11:28 UTC (permalink / raw)
To: stable
Cc: dragos.panait, wg, mkl, davem, paskripkin, gregkh, hbh25y,
linux-can, netdev, linux-kernel
The following commit is needed to fix CVE-2022-28388:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3d3925ff6433f98992685a9679613a2cc97f3ce2
Hangyu Hua (1):
can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in
error path
drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
base-commit: e7f5213d755bc34f366d36f08825c0b446117d96
--
2.17.1
^ permalink raw reply
* [PATCH 4.19 1/1] can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in error path
From: Dragos-Marian Panait @ 2022-04-19 11:36 UTC (permalink / raw)
To: stable
Cc: dragos.panait, wg, mkl, davem, paskripkin, gregkh, hbh25y,
linux-can, netdev, linux-kernel
In-Reply-To: <20220419113642.3115359-1-dragos.panait@windriver.com>
From: Hangyu Hua <hbh25y@gmail.com>
commit 3d3925ff6433f98992685a9679613a2cc97f3ce2 upstream.
There is no need to call dev_kfree_skb() when usb_submit_urb() fails
because can_put_echo_skb() deletes original skb and
can_free_echo_skb() deletes the cloned skb.
Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices")
Link: https://lore.kernel.org/all/20220311080614.45229-1-hbh25y@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
[DP: adjusted params of can_free_echo_skb() for 4.19 stable]
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
index df99354ec12a..232f45f722f0 100644
--- a/drivers/net/can/usb/usb_8dev.c
+++ b/drivers/net/can/usb/usb_8dev.c
@@ -681,9 +681,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
atomic_inc(&priv->active_tx_urbs);
err = usb_submit_urb(urb, GFP_ATOMIC);
- if (unlikely(err))
- goto failed;
- else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
+ if (unlikely(err)) {
+ can_free_echo_skb(netdev, context->echo_index);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
+
+ atomic_dec(&priv->active_tx_urbs);
+
+ if (err == -ENODEV)
+ netif_device_detach(netdev);
+ else
+ netdev_warn(netdev, "failed tx_urb %d\n", err);
+ stats->tx_dropped++;
+ } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
/* Slow down tx path */
netif_stop_queue(netdev);
@@ -702,19 +713,6 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
return NETDEV_TX_BUSY;
-failed:
- can_free_echo_skb(netdev, context->echo_index);
-
- usb_unanchor_urb(urb);
- usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
-
- atomic_dec(&priv->active_tx_urbs);
-
- if (err == -ENODEV)
- netif_device_detach(netdev);
- else
- netdev_warn(netdev, "failed tx_urb %d\n", err);
-
nomembuf:
usb_free_urb(urb);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v8 06/13] arm64: dts: freescale: Add i.MX8DXL evk board support
From: Krzysztof Kozlowski @ 2022-04-19 11:40 UTC (permalink / raw)
To: Abel Vesa, Rob Herring, Ulf Hansson, Shawn Guo, Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, Linux Kernel Mailing List, linux-mmc, netdev,
linux-arm-kernel, Jacky Bai
In-Reply-To: <20220419113516.1827863-7-abel.vesa@nxp.com>
On 19/04/2022 13:35, Abel Vesa wrote:
> From: Jacky Bai <ping.bai@nxp.com>
>
> Add i.MX8DXL EVK board support.
>
Thank you for your patch. There is something to discuss/improve.
(...)
> + /* global autoconfigured region for contiguous allocations */
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0 0x14000000>;
> + alloc-ranges = <0 0x98000000 0 0x14000000>;
> + linux,cma-default;
> + };
> + };
> +
> + reg_usdhc2_vmmc: usdhc2-vmmc {
usdhc2-vmmc-regulator or regulator-0
(...)
> +&thermal_zones {
> + pmic-thermal0 {
Are you sure this passes the dtbs_check?
> + polling-delay-passive = <250>;
> + polling-delay = <2000>;
> + thermal-sensors = <&tsens IMX_SC_R_PMIC_0>;
> +
(...)
> + pinctrl_usdhc1: usdhc1grp {
> + fsl,pins = <
> + IMX8DXL_EMMC0_CLK_CONN_EMMC0_CLK 0x06000041
> + IMX8DXL_EMMC0_CMD_CONN_EMMC0_CMD 0x00000021
> + IMX8DXL_EMMC0_DATA0_CONN_EMMC0_DATA0 0x00000021
> + IMX8DXL_EMMC0_DATA1_CONN_EMMC0_DATA1 0x00000021
> + IMX8DXL_EMMC0_DATA2_CONN_EMMC0_DATA2 0x00000021
> + IMX8DXL_EMMC0_DATA3_CONN_EMMC0_DATA3 0x00000021
> + IMX8DXL_EMMC0_DATA4_CONN_EMMC0_DATA4 0x00000021
> + IMX8DXL_EMMC0_DATA5_CONN_EMMC0_DATA5 0x00000021
> + IMX8DXL_EMMC0_DATA6_CONN_EMMC0_DATA6 0x00000021
> + IMX8DXL_EMMC0_DATA7_CONN_EMMC0_DATA7 0x00000021
> + IMX8DXL_EMMC0_STROBE_CONN_EMMC0_STROBE 0x00000041
> + >;
> + };
> +
> + pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
and this as well... I don't remember if we have schema for this but even
without it, it breaks the convention. See other files.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 5.4 1/1] can: usb_8dev: usb_8dev_start_xmit(): fix double dev_kfree_skb() in error path
From: Dragos-Marian Panait @ 2022-04-19 11:28 UTC (permalink / raw)
To: stable
Cc: dragos.panait, wg, mkl, davem, paskripkin, gregkh, hbh25y,
linux-can, netdev, linux-kernel
In-Reply-To: <20220419112821.3112299-1-dragos.panait@windriver.com>
From: Hangyu Hua <hbh25y@gmail.com>
commit 3d3925ff6433f98992685a9679613a2cc97f3ce2 upstream.
There is no need to call dev_kfree_skb() when usb_submit_urb() fails
because can_put_echo_skb() deletes original skb and
can_free_echo_skb() deletes the cloned skb.
Fixes: 0024d8ad1639 ("can: usb_8dev: Add support for USB2CAN interface from 8 devices")
Link: https://lore.kernel.org/all/20220311080614.45229-1-hbh25y@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
[DP: adjusted params of can_free_echo_skb() for 5.4 stable]
Signed-off-by: Dragos-Marian Panait <dragos.panait@windriver.com>
---
drivers/net/can/usb/usb_8dev.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
index c43e98bb6e2d..b514b2eaa318 100644
--- a/drivers/net/can/usb/usb_8dev.c
+++ b/drivers/net/can/usb/usb_8dev.c
@@ -670,9 +670,20 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
atomic_inc(&priv->active_tx_urbs);
err = usb_submit_urb(urb, GFP_ATOMIC);
- if (unlikely(err))
- goto failed;
- else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
+ if (unlikely(err)) {
+ can_free_echo_skb(netdev, context->echo_index);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
+
+ atomic_dec(&priv->active_tx_urbs);
+
+ if (err == -ENODEV)
+ netif_device_detach(netdev);
+ else
+ netdev_warn(netdev, "failed tx_urb %d\n", err);
+ stats->tx_dropped++;
+ } else if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
/* Slow down tx path */
netif_stop_queue(netdev);
@@ -691,19 +702,6 @@ static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
return NETDEV_TX_BUSY;
-failed:
- can_free_echo_skb(netdev, context->echo_index);
-
- usb_unanchor_urb(urb);
- usb_free_coherent(priv->udev, size, buf, urb->transfer_dma);
-
- atomic_dec(&priv->active_tx_urbs);
-
- if (err == -ENODEV)
- netif_device_detach(netdev);
- else
- netdev_warn(netdev, "failed tx_urb %d\n", err);
-
nomembuf:
usb_free_urb(urb);
--
2.17.1
^ permalink raw reply related
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