From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6AB32EB8FA5 for ; Wed, 6 Sep 2023 09:08:08 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4C6D8402B9; Wed, 6 Sep 2023 11:08:07 +0200 (CEST) Received: from smtpbg150.qq.com (smtpbg150.qq.com [18.132.163.193]) by mails.dpdk.org (Postfix) with ESMTP id EE0594029E for ; Wed, 6 Sep 2023 11:08:04 +0200 (CEST) X-QQ-mid: bizesmtp80t1693991273tuqcpjhq Received: from LAPTOP96V0OHHN ( [183.81.182.182]) by bizesmtp.qq.com (ESMTP) with id ; Wed, 06 Sep 2023 17:07:51 +0800 (CST) X-QQ-SSF: 00400000000000D0F000000A0000000 X-QQ-FEAT: D2GZf6M6C/ilym3Mc22FWngbjx2F8Hp96diNpqVeVEaz123AUCUm9E6+AJZL+ jFzHZflsMQvKVsIihjyXEEAXBsYs91pTT7rMCL/yBmF1PQ61rSRWd+FgnEMXubQ8tu7TUgx c/wOkneMjqFfIDj7Ki3PcU8DP9J6lcBJhEdLdIEnWflNU7CU//FBTc6je1VtQQTpsG4jFws g04UYf1TAf4sIbWr4rcKNeXY9oYD9AyIi9YGHqO4XGnljkCwolsBeJfgzOHUQozta5krR0k 7sqF4urw9rypvG1kp8GXVd7i/9mZ9c7VJp0DpAawTN2+AQ5SbyachjPV6YM3siTArbUkCWA k6AAqLP65eWnWguhKwn6vvs8ev/pD1NvnbNVCpaISQDBWCPPc5S+KmJq0TAicnLmQDz/EgA J9csSdoYCNs= X-QQ-GoodBg: 2 X-BIZMAIL-ID: 12033881988178445895 From: "11" To: "'Ferruh Yigit'" Cc: , , , References: <20230901023050.40893-1-caowenbo@mucse.com> <20230901023050.40893-7-caowenbo@mucse.com> In-Reply-To: Subject: RE: [PATCH v6 6/8] net/rnp add port info resource init Date: Wed, 6 Sep 2023 17:07:52 +0800 Message-ID: <378534F213285A8C+009501d9e0a1$9ef58fb0$dce0af10$@mucse.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Content-Language: zh-cn Thread-Index: AQI30P9XUj3aWZJ7n+uYhLh06w3v0AJzyGgxAwV4of2vJcugsA== X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:mucse.com:qybglogicsvrgz:qybglogicsvrgz5a-0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Ferruh, > -----Original Message----- > From: Ferruh Yigit > Sent: 2023=E5=B9=B49=E6=9C=886=E6=97=A5 0:56 > To: Wenbo Cao > Cc: dev@dpdk.org; thomas@monjalon.net; andrew.rybchenko@oktetlabs.ru; > yaojun@mucse.com > Subject: Re: [PATCH v6 6/8] net/rnp add port info resource init >=20 > On 9/1/2023 3:30 AM, Wenbo Cao wrote: > > Add Api For FW Mac Info, Port Resoucre info init Code For Different > > Shape Of Nic > > > > Signed-off-by: Wenbo Cao >=20 > <...> >=20 > > @@ -47,11 +104,53 @@ rnp_init_port_resource(struct rnp_eth_adapter > *adapter, > > uint8_t p_id) > > { > > struct rnp_eth_port *port =3D RNP_DEV_TO_PORT(dev); > > + struct rte_pci_device *pci_dev =3D adapter->pdev; > > + struct rnp_hw *hw =3D &adapter->hw; > > > > + port->adapt =3D adapter; > > + port->s_mode =3D adapter->s_mode; > > + port->port_stopped =3D 1; > > + port->hw =3D hw; > > port->eth_dev =3D dev; > > - adapter->ports[p_id] =3D port; > > + > > + dev->device =3D &pci_dev->device; > > + rte_eth_copy_pci_info(dev, pci_dev); > > dev->dev_ops =3D &rnp_eth_dev_ops; > > - RTE_SET_USED(name); > > + dev->rx_queue_count =3D rnp_dev_rx_queue_count; > > + dev->rx_descriptor_status =3D rnp_dev_rx_descriptor_status; > > + dev->tx_descriptor_status =3D rnp_dev_tx_descriptor_status; > > + dev->rx_pkt_burst =3D rnp_recv_pkts; > > + dev->tx_pkt_burst =3D rnp_xmit_pkts; > > + dev->tx_pkt_prepare =3D rnp_prep_pkts; > > + > > + rnp_setup_port_attr(port, dev, adapter->num_ports, p_id); > > + rnp_init_filter_setup(port, adapter->num_ports); > > + rnp_get_mac_addr(dev, port->mac_addr); > > + dev->data->mac_addrs =3D rte_zmalloc(name, sizeof(struct > rte_ether_addr) * > > + port->attr.max_mac_addrs, 0); > > + if (!dev->data->mac_addrs) { > > + RNP_PMD_DRV_LOG(ERR, "Memory allocation " > > + "for MAC failed! Exiting.\n"); > > + return -ENOMEM; > > + } > > + /* Allocate memory for storing hash filter MAC addresses */ > > + dev->data->hash_mac_addrs =3D rte_zmalloc(name, > > + RTE_ETHER_ADDR_LEN * port- > >attr.max_uc_mac_hash, 0); > > + if (dev->data->hash_mac_addrs =3D=3D NULL) { > > + RNP_PMD_INIT_LOG(ERR, "Failed to allocate %d bytes " > > + "needed to store MAC addresses", > > + RTE_ETHER_ADDR_LEN * port- > >attr.max_uc_mac_hash); > > + return -ENOMEM; >=20 > Should free 'dev->data->mac_addrs' here, or even better can be to = implement > 'rnp_dev_close()' to free device resources. >=20 >=20 > > + dev->data->mac_addrs =3D rte_zmalloc(name, sizeof(struct Yes, for the failed branch, need to free the last alloc success argument = mac_addrs > > + } > > + > > + rnp_set_default_mac(dev, port->mac_addr); >=20 > I guess the 'port->mac_addr' got from device via 'rnp_get_mac_addr()' > above, but if 'rnp_get_mac_addr()' fails what will be the value. = Should there be > check and set a random mac if required? >=20 Yes, you are right :) , it the rnp_get_mac_addr failed, this may have = fireware issue thing about=20 Communicate.This need to be check and warning. > > + rte_ether_addr_copy((const struct rte_ether_addr *)port->mac_addr, > > + dev->data->mac_addrs); > > + /* MTU */ > > + dev->data->mtu =3D RTE_ETHER_MAX_LEN - > > + RTE_ETHER_HDR_LEN - RTE_ETHER_CRC_LEN; > > + adapter->ports[p_id] =3D port; > > + rte_eth_dev_probing_finish(dev); > > >=20 > rte_eth_dev_probing_finish() is not required, as > 'rte_eth_dev_pci_generic_probe()' calls it if dev_init() returns = success. >=20 > <...> >=20 For N10 chip is just have two pcie-bdf, the one pcie-bdf may have = multiple port(max 4) 'rte_eth_dev_pci_generic_probe' probe the pcie-pdf port(master port port = 0) the other port alloc by 'rnp_alloc_eth_port', not set = 'rte_eth_dev_probing_finish()' so the 1,2,3 port will set 'rte_eth_dev_probing_finish()'. > > +static int32_t rnp_get_mac_addr_pf(struct rnp_eth_port *port, > > + uint8_t lane, > > + uint8_t *macaddr) > > +{ > > + struct rnp_hw *hw =3D RNP_DEV_TO_HW(port->eth_dev); > > + > > + return rnp_fw_get_macaddr(port->eth_dev, hw->pf_vf_num, macaddr, > > +lane); } > > + >=20 > These are mac_ops functions, normally the reason to have mac_ops is to = support > different HW that behaves slightly different and this difference = managed by > different function pointers per device. Is this your usecase? >=20 This is because for one pcie-bdf one port and one pcie-bdf multiple port = usecase For hareware design limit, we max can support eight port but just have = two pcie-bdf. We must use two set of api to resolve one pcie-bdf one port and one pcie-bdf multiple port for the port Control independence feature.=20 Such as vlan-filter, mac unicast/Multicast filter, promise, Multicast = promise, Broadcast promise. So that some API is common used for one-port-pcie-bdf and = multiple-port-one-pcie-bdf :(. And some mac-api must change when known the nic is multiple-port mode. > And since these are mac_ops, defined in the base folder header, it = suits better to > have separate .c file for them which is under base file, what do you = think? Or am > I getting these mac_ops wrong? >=20 You are right, =F0=9F=98=8A mac_ops api need to depart from rnp.c to a = separate.c . > > +static int32_t > > +rnp_set_default_mac_pf(struct rnp_eth_port *port, > > + uint8_t *mac) > > +{ > > + struct rnp_eth_adapter *adap =3D RNP_PORT_TO_ADAPTER(port); > > + uint16_t max_vfs; > > + > > + if (port->s_mode =3D=3D RNP_SHARE_INDEPENDENT) > > + return rnp_set_rafb(port->eth_dev, (uint8_t *)mac, > > + UINT8_MAX, 0); > > + > > + max_vfs =3D adap->max_vfs; > > + > > + return rnp_set_rafb(port->eth_dev, mac, max_vfs, 0); } > > + > > const struct rnp_mac_api rnp_mac_ops =3D { > > .reset_hw =3D rnp_reset_hw_pf, > > - .init_hw =3D rnp_init_hw_pf > > + .init_hw =3D rnp_init_hw_pf, > > + .get_mac_addr =3D rnp_get_mac_addr_pf, > > + .set_default_mac =3D rnp_set_default_mac_pf, > > + .set_rafb =3D rnp_set_mac_addr_pf, > > + .clear_rafb =3D rnp_clear_mac_addr_pf > > }; > > > > static void > > @@ -228,7 +434,11 @@ rnp_common_ops_init(struct rnp_eth_adapter > > *adapter) static int rnp_special_ops_init(struct rte_eth_dev > > *eth_dev) { > > - RTE_SET_USED(eth_dev); > > + struct rnp_eth_adapter *adapter =3D RNP_DEV_TO_ADAPTER(eth_dev); > > + struct rnp_share_ops *share_priv; > > + > > + share_priv =3D adapter->share_priv; > > + share_priv->mac_api =3D &rnp_mac_ops; > > >=20 > Can you please describe why this 'rnp_special_ops_init()' is for (its = difference > from rnp_common_ops_init()) ? >=20 >=20 In 'rnp_common_ops_init' both one pcie-bdf one port and one pcie-bdf = multiple port, will use mailbox to communicase with fireware. 'rnp_special_ops_init() is design to change the mac api when the nic is = multiple-port mode. For now, the code is just for one-pcie-one-port mode.=20