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 <caowe...@mucse.com>
<...>
> @@ -47,11 +104,53 @@ rnp_init_port_resource(struct rnp_eth_adapter *adapter,
> uint8_t p_id)
> {
> struct rnp_eth_port *port = RNP_DEV_TO_PORT(dev);
> + struct rte_pci_device *pci_dev = adapter->pdev;
> + struct rnp_hw *hw = &adapter->hw;
>
> + port->adapt = adapter;
> + port->s_mode = adapter->s_mode;
> + port->port_stopped = 1;
> + port->hw = hw;
> port->eth_dev = dev;
> - adapter->ports[p_id] = port;
> +
> + dev->device = &pci_dev->device;
> + rte_eth_copy_pci_info(dev, pci_dev);
> dev->dev_ops = &rnp_eth_dev_ops;
> - RTE_SET_USED(name);
> + dev->rx_queue_count = rnp_dev_rx_queue_count;
> + dev->rx_descriptor_status = rnp_dev_rx_descriptor_status;
> + dev->tx_descriptor_status = rnp_dev_tx_descriptor_status;
> + dev->rx_pkt_burst = rnp_recv_pkts;
> + dev->tx_pkt_burst = rnp_xmit_pkts;
> + dev->tx_pkt_prepare = 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 = 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 = rte_zmalloc(name,
> + RTE_ETHER_ADDR_LEN * port->attr.max_uc_mac_hash, 0);
> + if (dev->data->hash_mac_addrs == 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;
Should free 'dev->data->mac_addrs' here, or even better can be to
implement 'rnp_dev_close()' to free device resources.
> + }
> +
> + rnp_set_default_mac(dev, port->mac_addr);
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?
> + rte_ether_addr_copy((const struct rte_ether_addr *)port->mac_addr,
> + dev->data->mac_addrs);
> + /* MTU */
> + dev->data->mtu = RTE_ETHER_MAX_LEN -
> + RTE_ETHER_HDR_LEN - RTE_ETHER_CRC_LEN;
> + adapter->ports[p_id] = port;
> + rte_eth_dev_probing_finish(dev);
>
rte_eth_dev_probing_finish() is not required, as
'rte_eth_dev_pci_generic_probe()' calls it if dev_init() returns success.
<...>
> +static int32_t rnp_get_mac_addr_pf(struct rnp_eth_port *port,
> + uint8_t lane,
> + uint8_t *macaddr)
> +{
> + struct rnp_hw *hw = RNP_DEV_TO_HW(port->eth_dev);
> +
> + return rnp_fw_get_macaddr(port->eth_dev, hw->pf_vf_num, macaddr, lane);
> +}
> +
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?
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?
> +static int32_t
> +rnp_set_default_mac_pf(struct rnp_eth_port *port,
> + uint8_t *mac)
> +{
> + struct rnp_eth_adapter *adap = RNP_PORT_TO_ADAPTER(port);
> + uint16_t max_vfs;
> +
> + if (port->s_mode == RNP_SHARE_INDEPENDENT)
> + return rnp_set_rafb(port->eth_dev, (uint8_t *)mac,
> + UINT8_MAX, 0);
> +
> + max_vfs = adap->max_vfs;
> +
> + return rnp_set_rafb(port->eth_dev, mac, max_vfs, 0);
> +}
> +
> const struct rnp_mac_api rnp_mac_ops = {
> .reset_hw = rnp_reset_hw_pf,
> - .init_hw = rnp_init_hw_pf
> + .init_hw = rnp_init_hw_pf,
> + .get_mac_addr = rnp_get_mac_addr_pf,
> + .set_default_mac = rnp_set_default_mac_pf,
> + .set_rafb = rnp_set_mac_addr_pf,
> + .clear_rafb = 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 = RNP_DEV_TO_ADAPTER(eth_dev);
> + struct rnp_share_ops *share_priv;
> +
> + share_priv = adapter->share_priv;
> + share_priv->mac_api = &rnp_mac_ops;
>
Can you please describe why this 'rnp_special_ops_init()' is for (its
difference from rnp_common_ops_init()) ?
