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: <20231201203602.7e380716@kernel.org> Date: Fri, 1 Dec 2023 20:36:02 -0800 From: Jakub Kicinski <kuba@...nel.org> To: Justin Lai <justinlai0215@...ltek.com> Cc: <davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>, <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>, <andrew@...n.ch>, <pkshih@...ltek.com>, <larry.chiu@...ltek.com> Subject: Re: [PATCH net-next v13 01/13] rtase: Add pci table supported in this module On Thu, 30 Nov 2023 19:43:15 +0800 Justin Lai wrote: > + * Below is a simplified block diagram of the chip and its relevant interfaces. > + * > + * ************************* > + * * * > + * * CPU network device * > + * * * > + * * +-------------+ * > + * * | PCIE Host | * > + * ***********++************ > + * || > + * PCIE > + * || > + * ********************++********************** > + * * | PCIE Endpoint | * > + * * +---------------+ * > + * * | GMAC | * > + * * +--++--+ Realtek * > + * * || RTL90xx Series * > + * * || * > + * * +-------------++----------------+ * > + * * | | MAC | | * > + * * | +-----+ | * > + * * | | * > + * * | Ethernet Switch Core | * > + * * | | * > + * * | +-----+ +-----+ | * > + * * | | MAC |...........| MAC | | * > + * * +---+-----+-----------+-----+---+ * > + * * | PHY |...........| PHY | * > + * * +--++-+ +--++-+ * > + * *************||****************||*********** > + * > + * The block of the Realtek RTL90xx series is our entire chip architecture, > + * the GMAC is connected to the switch core, and there is no PHY in between. > + * In addition, this driver is mainly used to control GMAC, but does not > + * control the switch core, so it is not the same as DSA. Okay, but you seem to only register one netdev. Which MAC is it for? > +#include <linux/crc32.h> > +#include <linux/dma-mapping.h> > +#include <linux/etherdevice.h> > +#include <linux/if_vlan.h> > +#include <linux/in.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/ip.h> > +#include <linux/ipv6.h> > +#include <linux/mdio.h> > +#include <linux/module.h> > +#include <linux/moduleparam.h> I don't see module params, please trim the includes. > +#include <linux/netdevice.h> > +#include <linux/pci.h> > +#include <linux/pm_runtime.h> > +#include <linux/prefetch.h> > +#include <linux/rtnetlink.h> > +#include <linux/tcp.h> > +#include <asm/irq.h> > +#include <net/ip6_checksum.h> > +#include <net/page_pool/helpers.h> > +#include <net/pkt_cls.h> > +static void rtase_get_mac_address(struct net_device *dev) > +{ > + struct rtase_private *tp = netdev_priv(dev); > + u8 mac_addr[ETH_ALEN] __aligned(2) = {}; > + u32 i; > + > + for (i = 0; i < ETH_ALEN; i++) > + mac_addr[i] = rtase_r8(tp, RTASE_MAC0 + i); > + > + if (!is_valid_ether_addr(mac_addr)) { > + eth_random_addr(mac_addr); > + dev->addr_assign_type = NET_ADDR_RANDOM; eth_hw_addr_random() > + netdev_warn(dev, "Random ether addr %pM\n", mac_addr); > + } > + > + eth_hw_addr_set(dev, mac_addr); > + rtase_rar_set(tp, mac_addr); > + > + ether_addr_copy(dev->perm_addr, dev->dev_addr); Should it be perm if it's random? > +} > + > +static void rtase_reset_interrupt(struct pci_dev *pdev, > + const struct rtase_private *tp) > +{ > + if (tp->sw_flag & SWF_MSIX_ENABLED) > + pci_disable_msix(pdev); > + else > + pci_disable_msi(pdev); > +} > + > +static int rtase_alloc_msix(struct pci_dev *pdev, struct rtase_private *tp) > +{ > + int ret; > + u16 i; > + > + memset(tp->msix_entry, 0x0, RTASE_NUM_MSIX * sizeof(struct msix_entry)); > + > + for (i = 0; i < RTASE_NUM_MSIX; i++) > + tp->msix_entry[i].entry = i; > + > + ret = pci_enable_msix_range(pdev, tp->msix_entry, tp->int_nums, > + tp->int_nums); pci_enable_msix_exact() > + if (ret == tp->int_nums) { > + for (i = 0; i < tp->int_nums; i++) { > + tp->int_vector[i].irq = pci_irq_vector(pdev, i); > + tp->int_vector[i].status = 1; > + } > + } > + > + return ret; > +} > + if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) > + dev->features |= NETIF_F_HIGHDMA; > + else if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) > + goto err_out_free_res; > + else > + dev_info(&pdev->dev, "DMA_BIT_MASK: 32\n"); This dance is unnecessary, see https://lkml.org/lkml/2021/6/7/398 > + dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats); > + if (!dev->tstats) > + goto err_out_1; Please use dev->pcpu_stat_type Set it before register and core will allocate stats for you. > + ret = register_netdev(dev); > + if (ret != 0) > + goto err_out; > + > + netdev_dbg(dev, "%pM, IRQ %d\n", dev->dev_addr, dev->irq); > + > + netif_carrier_off(dev); Should be before register_netdev(). > + goto out; Just return 0... > +static void rtase_remove_one(struct pci_dev *pdev) > +{ > + struct net_device *dev = pci_get_drvdata(pdev); > + struct rtase_private *tp = netdev_priv(dev); > + struct rtase_int_vector *ivec; > + u32 i; > + > + for (i = 0; i < tp->int_nums; i++) { > + ivec = &tp->int_vector[i]; > + netif_napi_del(&ivec->napi); NAPI instances should be added on ndo_open()
Powered by blists - more mailing lists