netdev - Re: [PATCH net-next v5 08/10] net: libwx: add eswitch switch api for devlink ops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <721C2E58-6348-4645-A932-0FF6499147B1@net-swift.com>
Date: Wed, 7 Aug 2024 15:41:53 +0800
From: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Simon Horman <horms@...nel.org>,
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5 08/10] net: libwx: add eswitch switch api for
 devlink ops



> 2024年8月6日 19:42，Jiri Pirko <jiri@...nulli.us> 写道：
> 
> Mon, Aug 05, 2024 at 06:43:50PM CEST, horms@...nel.org wrote:
>> On Sun, Aug 04, 2024 at 08:48:39PM +0800, Mengyuan Lou wrote:
>> 
>> Each patch needs a patch description describing not just what is done
>> but why.
>> 
>> Also, please seed the CC list for patch submissions
>> using get_maintainer this.patch. I believe that b4
>> will do that for you.
>> 
>>> Signed-off-by: Mengyuan Lou <mengyuanlou@...-swift.com>
>> 
>> ...
>> 
>>> static void wx_devlink_free(void *devlink_ptr)
>>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_eswitch.c b/drivers/net/ethernet/wangxun/libwx/wx_eswitch.c
>>> new file mode 100644
>>> index 000000000000..a426a352bf96
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_eswitch.c
>>> @@ -0,0 +1,53 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Copyright (C) 2019-2021, Intel Corporation. */
>> 
>> Are you sure Intel holds the copyright on this code?
>> 
>>> +
>>> +#include <linux/pci.h>
>>> +
>>> +#include "wx_type.h"
>>> +#include "wx_eswitch.h"
>>> +#include "wx_devlink.h"
>>> +
>>> +int wx_eswitch_mode_set(struct devlink *devlink, u16 mode,
>>> + struct netlink_ext_ack *extack)
>>> +{
>>> + struct wx_dl_priv *dl_priv = devlink_priv(devlink);
>>> + struct wx *wx = dl_priv->priv_wx;
>>> +
>>> + if (wx->eswitch_mode == mode)
>>> + return 0;
>>> +
>>> + if (wx->num_vfs) {
>>> + dev_info(&(wx)->pdev->dev,
>>> + "Change eswitch mode is allowed if there is no VFs.");
>> 
>> maybe: Changing eswitch mode is only allowed if there are no VFs.
>> 
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + switch (mode) {
>>> + case DEVLINK_ESWITCH_MODE_LEGACY:
>>> + dev_info(&(wx)->pdev->dev,
>>> + "PF%d changed eswitch mode to legacy",
>>> + wx->bus.func);
>>> + NL_SET_ERR_MSG_MOD(extack, "Changed eswitch mode to legacy");
>>> + break;
>>> + case DEVLINK_ESWITCH_MODE_SWITCHDEV:
>>> + dev_info(&(wx)->pdev->dev,
>>> + "Do not support switchdev in eswitch mode.");
>>> + NL_SET_ERR_MSG_MOD(extack, "Do not support switchdev mode.");
>> 
>> maybe: eswitch mode switchdev is not supported
>> 
>> I am curious to know if you are planning to implement eswitch mode in the
>> near future.  If not, is wx_eswitch_mode_set() needed: it seems unused in
>> this patchset: it should probably be added in a patchset that uses it.
> 
> This is wrong.
> 
> switchdev mode should be supported from the beginning. There is some odd
> hybrid mode supported, VF gets devlink port created for representors,
> yet no netdev. I'm very sure this triggers:
> 
> static void devlink_port_type_warn(struct work_struct *work)
> {
>        struct devlink_port *port = container_of(to_delayed_work(work),
>                                                 struct devlink_port,
>                                                 type_warn_dw);
>        dev_warn(port->devlink->dev, "Type was not set for devlink port.");
> }
> 
> So, if you don't see this message, leads to my assuptions this patchset
> was not tested. Certainly odd.
> 

I have seen this warnning info.
What I want is that the devlink port Uapi to vfs, but I think the product will
not to support switchdev mode and the representors representors will not be used.

Be like:
https://lore.kernel.org/netdev/20240620002741.1029936-1-kuba@kernel.org/

> 
>> 
>>> + return -EINVAL;
>>> + default:
>>> + NL_SET_ERR_MSG_MOD(extack, "Unknown eswitch mode");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + wx->eswitch_mode = mode;
>>> + return 0;
>>> +}
>>> +
>>> +int wx_eswitch_mode_get(struct devlink *devlink, u16 *mode)
>>> +{
>>> + struct wx_dl_priv *dl_priv = devlink_priv(devlink);
>>> + struct wx *wx = dl_priv->priv_wx;
>>> +
>>> + *mode = wx->eswitch_mode;
>>> + return 0;
>>> +}
>> 
>> ...
>> 
>