| 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
| ||
|
Message-ID: <CALs4sv28UttVJh2vCbax=foQYu83Qn1NsiUWOGQhKHsKg7ZwqA@mail.gmail.com>
Date: Wed, 15 Oct 2025 15:59:36 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Fan Gong <gongfan1@...wei.com>
Cc: Zhu Yikai <zhuyikai1@...artners.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>, linux-doc@...r.kernel.org,
Jonathan Corbet <corbet@....net>, Bjorn Helgaas <helgaas@...nel.org>, luosifu <luosifu@...wei.com>,
Xin Guo <guoxin09@...wei.com>, Shen Chenyang <shenchenyang1@...ilicon.com>,
Zhou Shuai <zhoushuai28@...wei.com>, Wu Like <wulike1@...wei.com>,
Shi Jing <shijing34@...wei.com>, Luo Yang <luoyang82@...artners.com>,
Meny Yossefi <meny.yossefi@...wei.com>, Gur Stavi <gur.stavi@...wei.com>,
Lee Trager <lee@...ger.us>, Michael Ellerman <mpe@...erman.id.au>,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Suman Ghosh <sumang@...vell.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>, Joe Damato <jdamato@...tly.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>
Subject: Re: [PATCH net-next v01 1/9] hinic3: Add PF framework
On Wed, Oct 15, 2025 at 12:48 PM Fan Gong <gongfan1@...wei.com> wrote:
>
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.c
> @@ -314,6 +314,9 @@ int hinic3_func_rx_tx_flush(struct hinic3_hwdev *hwdev)
> ret = -EFAULT;
> }
>
> + if (HINIC3_FUNC_TYPE(hwdev) != HINIC3_FUNC_TYPE_VF)
Is this check needed? Other places hinic3_set_pf_status() is called
without checking. I see the function has an internal check for PF.
> + hinic3_set_pf_status(hwif, HINIC3_PF_STATUS_FLR_START_FLAG);
> +
> clr_res.func_id = hwif->attr.func_global_idx;
> msg_params.buf_in = &clr_res;
> msg_params.in_size = sizeof(clr_res);
> @@ -337,6 +340,54 @@ int hinic3_func_rx_tx_flush(struct hinic3_hwdev *hwdev)
> return ret;
> }
>
> +int hinic3_set_bdf_ctxt(struct hinic3_hwdev *hwdev, u8 bus, u8 device, u8 function)
You could just send *bdf_info as a parameter with values pre-set,
makes it look better
> +{
> + struct comm_cmd_bdf_info bdf_info = {};
> + struct mgmt_msg_params msg_params = {};
> + int err;
> +
> + bdf_info.function_idx = hinic3_global_func_id(hwdev);
> + bdf_info.bus = bus;
> + bdf_info.device = device;
> + bdf_info.function = function;
> +
> + mgmt_msg_params_init_default(&msg_params, &bdf_info, sizeof(bdf_info));
> +
> + err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_COMM,
> + COMM_CMD_SEND_BDF_INFO, &msg_params);
> + if (err || bdf_info.head.status) {
> + dev_err(hwdev->dev,
> + "Failed to set bdf info to fw, err: %d, status: 0x%x\n",
> + err, bdf_info.head.status);
> + return -EIO;
> + }
> +
> + return 0;
> +}
<-->
> +static void set_ppf(struct hinic3_hwdev *hwdev)
Any particular reason why this function does not have hinic3 prefix?
> +{
> + struct hinic3_hwif *hwif = hwdev->hwif;
> + struct hinic3_func_attr *attr;
> + u32 addr, val;
> +
> + if (HINIC3_IS_VF(hwdev))
> + return;
> +
> + /* Read Modify Write */
> + attr = &hwif->attr;
> + addr = HINIC3_CSR_PPF_ELECTION_ADDR;
> + val = hinic3_hwif_read_reg(hwif, addr);
> + val &= ~HINIC3_PPF_ELECTION_IDX_MASK;
> + val |= HINIC3_PPF_ELECTION_SET(attr->func_global_idx, IDX);
> + hinic3_hwif_write_reg(hwif, addr, val);
> +
> + /* Check PPF index */
> + val = hinic3_hwif_read_reg(hwif, addr);
> + attr->ppf_idx = HINIC3_PPF_ELECTION_GET(val, IDX);
> +}
> +
<-->
> +int hinic3_get_default_mac(struct hinic3_hwdev *hwdev, u8 *mac_addr)
> +{
> + struct l2nic_cmd_set_mac mac_info = {};
> + struct mgmt_msg_params msg_params = {};
> + int err;
> +
> + mac_info.func_id = hinic3_global_func_id(hwdev);
> +
> + mgmt_msg_params_init_default(&msg_params, &mac_info, sizeof(mac_info));
> +
> + err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC,
> + L2NIC_CMD_GET_MAC,
> + &msg_params);
> +
> + if (err || mac_info.msg_head.status) {
> + dev_err(hwdev->dev,
> + "Failed to get mac, err: %d, status: 0x%x\n",
> + err, mac_info.msg_head.status);
> + return -EINVAL;
EINVAL looks odd. I see other places it is -EIO.
> + }
> +
> + ether_addr_copy(mac_addr, mac_info.mac);
> +
> + return 0;
> +}
> +
> int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id,
> u16 func_id)
> {
> @@ -157,9 +189,9 @@ int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id,
> return -EIO;
> }
>
> - if (mac_info.msg_head.status == MGMT_STATUS_PF_SET_VF_ALREADY) {
> + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) {
> dev_warn(hwdev->dev, "PF has already set VF mac, Ignore set operation\n");
> - return 0;
> + return HINIC3_PF_SET_VF_ALREADY;
> }
>
> if (mac_info.msg_head.status == MGMT_STATUS_EXIST) {
> @@ -191,11 +223,17 @@ int hinic3_del_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id,
>
> err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC,
> L2NIC_CMD_DEL_MAC, &msg_params);
> - if (err) {
> + if (err || (mac_info.msg_head.status &&
> + !PF_SET_VF_MAC(hwdev, mac_info.msg_head.status))) {
> dev_err(hwdev->dev,
> "Failed to delete MAC, err: %d, status: 0x%x\n",
> err, mac_info.msg_head.status);
> - return err;
> + return -EIO;
> + }
> +
> + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) {
> + dev_warn(hwdev->dev, "PF has already set VF mac, Ignore delete operation.\n");
> + return HINIC3_PF_SET_VF_ALREADY;
> }
>
> return 0;
> @@ -231,6 +269,17 @@ int hinic3_update_mac(struct hinic3_hwdev *hwdev, const u8 *old_mac,
> return -EIO;
> }
>
> + if (PF_SET_VF_MAC(hwdev, mac_info.msg_head.status)) {
> + dev_warn(hwdev->dev, "PF has already set VF MAC. Ignore update operation\n");
> + return HINIC3_PF_SET_VF_ALREADY;
> + }
> +
> + if (mac_info.msg_head.status == HINIC3_MGMT_STATUS_EXIST) {
> + dev_warn(hwdev->dev,
> + "MAC is repeated. Ignore update operation\n");
> + return 0;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
> index b83b567fa542..08bf14679bf8 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
> @@ -16,6 +16,8 @@ struct hinic3_nic_dev;
> #define HINIC3_MAX_JUMBO_FRAME_SIZE 9600
>
> #define HINIC3_VLAN_ID_MASK 0x7FFF
> +#define HINIC3_PF_SET_VF_ALREADY 0x4
> +#define HINIC3_MGMT_STATUS_EXIST 0x6
>
> enum hinic3_nic_event_type {
> HINIC3_NIC_EVENT_LINK_DOWN = 0,
> @@ -41,6 +43,7 @@ void hinic3_update_nic_feature(struct hinic3_nic_dev *nic_dev, u64 feature_cap);
> int hinic3_init_function_table(struct hinic3_nic_dev *nic_dev);
> int hinic3_set_port_mtu(struct net_device *netdev, u16 new_mtu);
>
> +int hinic3_get_default_mac(struct hinic3_hwdev *hwdev, u8 *mac_addr);
> int hinic3_set_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id,
> u16 func_id);
> int hinic3_del_mac(struct hinic3_hwdev *hwdev, const u8 *mac_addr, u16 vlan_id,
> --
> 2.43.0
>
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5469 bytes)
Powered by blists - more mailing lists