* Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support
From: Saeed Mahameed @ 2019-07-23 21:38 UTC (permalink / raw)
To: peter_hong@fintek.com.tw, wg@grandegger.com, mkl@pengutronix.de,
hpeter@gmail.com
Cc: hpeter+linux_kernel@gmail.com, f.suligoi@asem.it,
linux-can@vger.kernel.org, davem@davemloft.net,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1563776521-28317-1-git-send-email-hpeter+linux_kernel@gmail.com>
On Mon, 2019-07-22 at 14:22 +0800, Ji-Ze Hong (Peter Hong) wrote:
> This patch add support for Fintek PCIE to 2 CAN controller support
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com
> >
> ---
> Changelog:
> v2:
> 1: Fix comment on the spinlock with write access.
> 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
> 3: Check the strap pin outside the loop.
> 4: Fix the cleanup issue in f81601_pci_add_card().
> 5: Remove unused "channels" in struct f81601_pci_card.
>
> drivers/net/can/sja1000/Kconfig | 8 ++
> drivers/net/can/sja1000/Makefile | 1 +
> drivers/net/can/sja1000/f81601.c | 215
> +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 224 insertions(+)
> create mode 100644 drivers/net/can/sja1000/f81601.c
>
> diff --git a/drivers/net/can/sja1000/Kconfig
> b/drivers/net/can/sja1000/Kconfig
> index f6dc89927ece..8588323c5138 100644
> --- a/drivers/net/can/sja1000/Kconfig
> +++ b/drivers/net/can/sja1000/Kconfig
> @@ -101,4 +101,12 @@ config CAN_TSCAN1
> IRQ numbers are read from jumpers JP4 and JP5,
> SJA1000 IO base addresses are chosen heuristically (first
> that works).
>
> +config CAN_F81601
> + tristate "Fintek F81601 PCIE to 2 CAN Controller"
> + depends on PCI
> + help
> + This driver adds support for Fintek F81601 PCIE to 2 CAN
> Controller.
> + It had internal 24MHz clock source, but it can be changed by
> + manufacturer. We can use modinfo to get usage for parameters.
> + Visit http://www.fintek.com.tw to get more information.
> endif
> diff --git a/drivers/net/can/sja1000/Makefile
> b/drivers/net/can/sja1000/Makefile
> index 9253aaf9e739..6f6268543bd9 100644
> --- a/drivers/net/can/sja1000/Makefile
> +++ b/drivers/net/can/sja1000/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_CAN_PEAK_PCMCIA) += peak_pcmcia.o
> obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
> obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
> obj-$(CONFIG_CAN_TSCAN1) += tscan1.o
> +obj-$(CONFIG_CAN_F81601) += f81601.o
> diff --git a/drivers/net/can/sja1000/f81601.c
> b/drivers/net/can/sja1000/f81601.c
> new file mode 100644
> index 000000000000..3c378de8764d
> --- /dev/null
> +++ b/drivers/net/can/sja1000/f81601.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Fintek F81601 PCIE to 2 CAN controller driver
> + *
> + * Copyright (C) 2019 Peter Hong <peter_hong@fintek.com.tw>
> + * Copyright (C) 2019 Linux Foundation
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/netdevice.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +#include <linux/can/dev.h>
> +#include <linux/io.h>
> +#include <linux/version.h>
> +
> +#include "sja1000.h"
> +
> +#define F81601_PCI_MAX_CHAN 2
> +
> +#define F81601_DECODE_REG 0x209
> +#define F81601_IO_MODE BIT(7)
> +#define F81601_MEM_MODE BIT(6)
> +#define F81601_CFG_MODE BIT(5)
> +#define F81601_CAN2_INTERNAL_CLK BIT(3)
> +#define F81601_CAN1_INTERNAL_CLK BIT(2)
> +#define F81601_CAN2_EN BIT(1)
> +#define F81601_CAN1_EN BIT(0)
> +
> +#define F81601_TRAP_REG 0x20a
> +#define F81601_CAN2_HAS_EN BIT(4)
> +
> +struct f81601_pci_card {
> + void __iomem *addr;
> + spinlock_t lock; /* use this spin lock only for write access
> */
> + struct pci_dev *dev;
> + struct net_device *net_dev[F81601_PCI_MAX_CHAN];
> +};
> +
> +static const struct pci_device_id f81601_pci_tbl[] = {
> + { PCI_DEVICE(0x1c29, 0x1703) },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(pci, f81601_pci_tbl);
> +
> +static bool internal_clk = 1;
> +module_param(internal_clk, bool, 0444);
> +MODULE_PARM_DESC(internal_clk, "Use internal clock, default 1
> (24MHz)");
> +
> +static unsigned int external_clk;
> +module_param(external_clk, uint, 0444);
> +MODULE_PARM_DESC(external_clk, "External Clock, must spec when
> internal_clk = 0");
> +
> +static u8 f81601_pci_read_reg(const struct sja1000_priv *priv, int
> port)
> +{
> + return readb(priv->reg_base + port);
> +}
> +
> +static void f81601_pci_write_reg(const struct sja1000_priv *priv,
> int port,
> + u8 val)
> +{
> + struct f81601_pci_card *card = priv->priv;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&card->lock, flags);
> + writeb(val, priv->reg_base + port);
> + readb(priv->reg_base);
> + spin_unlock_irqrestore(&card->lock, flags);
> +}
> +
> +static void f81601_pci_del_card(struct pci_dev *pdev)
> +{
> + struct f81601_pci_card *card = pci_get_drvdata(pdev);
> + struct net_device *dev;
> + int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
> + dev = card->net_dev[i];
> + if (!dev)
> + continue;
> +
> + dev_info(&pdev->dev, "%s: Removing %s\n", __func__,
> dev->name);
> +
> + unregister_sja1000dev(dev);
> + free_sja1000dev(dev);
> + }
> +
> + pcim_iounmap(pdev, card->addr);
> +}
> +
> +/* Probe F81601 based device for the SJA1000 chips and register each
> + * available CAN channel to SJA1000 Socket-CAN subsystem.
> + */
> +static int f81601_pci_add_card(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct sja1000_priv *priv;
> + struct net_device *dev;
> + struct f81601_pci_card *card;
nit, reverse xmas tree.
> + int err, i, count;
> + u8 tmp;
> +
> + if (pcim_enable_device(pdev) < 0) {
> + dev_err(&pdev->dev, "Failed to enable PCI device\n");
> + return -ENODEV;
> + }
> +
> + dev_info(&pdev->dev, "Detected card at slot #%i\n",
> + PCI_SLOT(pdev->devfn));
> +
> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> + if (!card)
> + return -ENOMEM;
> +
> + card->dev = pdev;
> + spin_lock_init(&card->lock);
> +
> + pci_set_drvdata(pdev, card);
> +
> + tmp = F81601_IO_MODE | F81601_MEM_MODE | F81601_CFG_MODE |
> + F81601_CAN2_EN | F81601_CAN1_EN;
> +
> + if (internal_clk) {
> + tmp |= F81601_CAN2_INTERNAL_CLK |
> F81601_CAN1_INTERNAL_CLK;
> +
> + dev_info(&pdev->dev,
> + "F81601 running with internal clock:
> 24Mhz\n");
> + } else {
> + dev_info(&pdev->dev,
> + "F81601 running with external clock: %dMhz\n",
> + external_clk / 1000000);
> + }
> +
> + pci_write_config_byte(pdev, F81601_DECODE_REG, tmp);
> +
> + card->addr = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
> +
> + if (!card->addr) {
> + err = -ENOMEM;
> + dev_err(&pdev->dev, "%s: Failed to remap BAR\n",
> __func__);
> + goto failure_cleanup;
> + }
> +
> + /* read CAN2_HW_EN strap pin to detect how many CANBUS do we
> have */
> + count = ARRAY_SIZE(card->net_dev);
> + pci_read_config_byte(pdev, F81601_TRAP_REG, &tmp);
> + if (!(tmp & F81601_CAN2_HAS_EN))
> + count = 1;
> +
> + /* Detect available channels */
> + for (i = 0; i < count; i++) {
> + dev = alloc_sja1000dev(0);
> + if (!dev) {
> + err = -ENOMEM;
> + goto failure_cleanup;
> + }
> +
don't you need to rollback and cleanup/unregister previously allocated
devs ?
> + priv = netdev_priv(dev);
> + priv->priv = card;
> + priv->irq_flags = IRQF_SHARED;
> + priv->reg_base = card->addr + 0x80 * i;
> + priv->read_reg = f81601_pci_read_reg;
> + priv->write_reg = f81601_pci_write_reg;
> +
> + if (internal_clk)
> + priv->can.clock.freq = 24000000 / 2;
> + else
> + priv->can.clock.freq = external_clk / 2;
> +
> + priv->ocr = OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL;
> + priv->cdr = CDR_CBP;
> +
> + SET_NETDEV_DEV(dev, &pdev->dev);
> + dev->dev_id = i;
> + dev->irq = pdev->irq;
> +
> + /* Register SJA1000 device */
> + err = register_sja1000dev(dev);
> + if (err) {
> + dev_err(&pdev->dev,
> + "%s: Registering device failed: %x\n",
> __func__,
> + err);
> + free_sja1000dev(dev);
> + goto failure_cleanup;
> + }
> +
> + card->net_dev[i] = dev;
> + dev_info(&pdev->dev, "Channel #%d, %s at 0x%p, irq
> %d\n", i,
> + dev->name, priv->reg_base, dev->irq);
> + }
> +
> + return 0;
> +
> +failure_cleanup:
> + dev_err(&pdev->dev, "%s: failed: %d. Cleaning Up.\n", __func__,
> err);
> + f81601_pci_del_card(pdev);
> +
> + return err;
> +}
> +
> +static struct pci_driver f81601_pci_driver = {
> + .name = "f81601",
> + .id_table = f81601_pci_tbl,
> + .probe = f81601_pci_add_card,
> + .remove = f81601_pci_del_card,
> +};
> +
> +MODULE_DESCRIPTION("Fintek F81601 PCIE to 2 CANBUS adaptor driver");
> +MODULE_AUTHOR("Peter Hong <peter_hong@fintek.com.tw>");
> +MODULE_LICENSE("GPL v2");
> +
> +module_pci_driver(f81601_pci_driver);
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jon Hunter @ 2019-07-23 21:39 UTC (permalink / raw)
To: Robin Murphy, Jose Abreu, Lars Persson, Ilias Apalodimas
Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com, Chen-Yu Tsai,
Maxime Coquelin, linux-tegra, Giuseppe Cavallaro,
David S . Miller, linux-arm-kernel@lists.infradead.org
In-Reply-To: <3255edfa-4465-204b-4751-8d40c8fb1382@arm.com>
On 23/07/2019 14:19, Robin Murphy wrote:
...
>>> Do you know if the SMMU interrupts are working correctly? If not, it's
>>> possible that an incorrect address or mapping direction could lead to
>>> the DMA transaction just being silently terminated without any fault
>>> indication, which generally presents as inexplicable weirdness (I've
>>> certainly seen that on another platform with the mix of an unsupported
>>> interrupt controller and an 'imperfect' ethernet driver).
>>
>> If I simply remove the iommu node for the ethernet controller, then I
>> see lots of ...
>>
>> [ 6.296121] arm-smmu 12000000.iommu: Unexpected global fault, this
>> could be serious
>> [ 6.296125] arm-smmu 12000000.iommu: GFSR 0x00000002,
>> GFSYNR0 0x00000000, GFSYNR1 0x00000014, GFSYNR2 0x00000000
>>
>> So I assume that this is triggering the SMMU interrupt correctly.
>
> According to tegra186.dtsi it appears you're using the MMU-500 combined
> interrupt, so if global faults are being delivered then context faults
> *should* also, but I'd be inclined to try a quick hack of the relevant
> stmmac_desc_ops::set_addr callback to write some bogus unmapped address
> just to make sure arm_smmu_context_fault() then screams as expected, and
> we're not missing anything else.
I hacked the driver and forced the address to zero for a test and
in doing so I see ...
[ 10.440072] arm-smmu 12000000.iommu: Unhandled context fault: fsr=0x402, iova=0x00000000, fsynr=0x1c0011, cbfrsynra=0x14, cb=0
So looks like the interrupts are working AFAICT.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v4 net-next 18/19] ionic: Add coalesce and other features
From: David Miller @ 2019-07-23 21:40 UTC (permalink / raw)
To: snelson; +Cc: netdev
In-Reply-To: <20190722214023.9513-19-snelson@pensando.io>
From: Shannon Nelson <snelson@pensando.io>
Date: Mon, 22 Jul 2019 14:40:22 -0700
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 742d7d47f4d8..e6b579a40b70 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -377,6 +377,75 @@ static int ionic_get_coalesce(struct net_device *netdev,
> return 0;
> }
>
> +static int ionic_set_coalesce(struct net_device *netdev,
> + struct ethtool_coalesce *coalesce)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + struct identity *ident = &lif->ionic->ident;
> + struct ionic_dev *idev = &lif->ionic->idev;
> + u32 tx_coal, rx_coal;
> + struct qcq *qcq;
> + unsigned int i;
Reverse christmas tree please.
^ permalink raw reply
* Re: [PATCH v4 net-next 19/19] ionic: Add basic devlink interface
From: David Miller @ 2019-07-23 21:40 UTC (permalink / raw)
To: snelson; +Cc: netdev
In-Reply-To: <20190722214023.9513-20-snelson@pensando.io>
From: Shannon Nelson <snelson@pensando.io>
Date: Mon, 22 Jul 2019 14:40:23 -0700
> +struct ionic *ionic_devlink_alloc(struct device *dev)
> +{
> + struct devlink *dl;
> + struct ionic *ionic;
Reverse christmas tree please.
^ permalink raw reply
* [PATCH bpf-next] libbpf: silence GCC8 warning about string truncation
From: Andrii Nakryiko @ 2019-07-23 22:01 UTC (permalink / raw)
To: bpf, netdev, ast, daniel
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Magnus Karlsson
Despite a proper NULL-termination after strncpy(..., ..., IFNAMSIZ - 1),
GCC8 still complains about *expected* string truncation:
xsk.c:330:2: error: 'strncpy' output may be truncated copying 15 bytes
from a string of length 15 [-Werror=stringop-truncation]
strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
This patch gets rid of the issue altogether by using memcpy instead.
There is no performance regression, as strncpy will still copy and fill
all of the bytes anyway.
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/xsk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 5007b5d4fd2c..65f5dd556f99 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
channels.cmd = ETHTOOL_GCHANNELS;
ifr.ifr_data = (void *)&channels;
- strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
+ memcpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
ifr.ifr_name[IFNAMSIZ - 1] = '\0';
err = ioctl(fd, SIOCETHTOOL, &ifr);
if (err && errno != EOPNOTSUPP) {
@@ -517,7 +517,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
err = -errno;
goto out_socket;
}
- strncpy(xsk->ifname, ifname, IFNAMSIZ - 1);
+ memcpy(xsk->ifname, ifname, IFNAMSIZ - 1);
xsk->ifname[IFNAMSIZ - 1] = '\0';
err = xsk_set_xdp_socket_config(&xsk->config, usr_config);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 1/5] selftests/bpf: convert test_get_stack_raw_tp to perf_buffer API
From: Song Liu @ 2019-07-23 22:02 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, daniel@iogearbox.net,
andrii.nakryiko@gmail.com, Kernel Team
In-Reply-To: <20190723213445.1732339-2-andriin@fb.com>
> On Jul 23, 2019, at 2:34 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>
> Convert test_get_stack_raw_tp test to new perf_buffer API.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply
* Re: [PATCH v2 bpf-next] libbpf: provide more helpful message on uninitialized global var
From: Song Liu @ 2019-07-23 22:03 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, netdev@vger.kernel.org, Alexei Starovoitov,
daniel@iogearbox.net, andrii.nakryiko@gmail.com, Kernel Team
In-Reply-To: <20190723211133.1666218-1-andriin@fb.com>
> On Jul 23, 2019, at 2:11 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>
> When BPF program defines uninitialized global variable, it's put into
> a special COMMON section. Libbpf will reject such programs, but will
> provide very unhelpful message with garbage-looking section index.
>
> This patch detects special section cases and gives more explicit error
> message.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 794dd5064ae8..8741c39adb1c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1760,15 +1760,22 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
> (long long) sym.st_value, sym.st_name, name);
>
> shdr_idx = sym.st_shndx;
> + insn_idx = rel.r_offset / sizeof(struct bpf_insn);
> + pr_debug("relocation: insn_idx=%u, shdr_idx=%u\n",
> + insn_idx, shdr_idx);
> +
> + if (shdr_idx >= SHN_LORESERVE) {
> + pr_warning("relocation: not yet supported relo for non-static global \'%s\' variable in special section (0x%x) found in insns[%d].code 0x%x\n",
> + name, shdr_idx, insn_idx,
> + insns[insn_idx].code);
> + return -LIBBPF_ERRNO__RELOC;
> + }
> if (!bpf_object__relo_in_known_section(obj, shdr_idx)) {
> pr_warning("Program '%s' contains unrecognized relo data pointing to section %u\n",
> prog->section_name, shdr_idx);
> return -LIBBPF_ERRNO__RELOC;
> }
>
> - insn_idx = rel.r_offset / sizeof(struct bpf_insn);
> - pr_debug("relocation: insn_idx=%u\n", insn_idx);
> -
> if (insns[insn_idx].code == (BPF_JMP | BPF_CALL)) {
> if (insns[insn_idx].src_reg != BPF_PSEUDO_CALL) {
> pr_warning("incorrect bpf_call opcode\n");
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] st21nfca_connectivity_event_received: null check the allocation
From: Navid Emamdoost @ 2019-07-23 22:04 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, secalert, Navid Emamdoost,
Thomas Gleixner, Kate Stewart, Allison Randal, Greg Kroah-Hartman,
netdev, linux-kernel
devm_kzalloc may fail and return null. So the null check is needed.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/nfc/st21nfca/se.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nfc/st21nfca/se.c b/drivers/nfc/st21nfca/se.c
index 06fc542fd198..6586378cacb0 100644
--- a/drivers/nfc/st21nfca/se.c
+++ b/drivers/nfc/st21nfca/se.c
@@ -317,6 +317,8 @@ int st21nfca_connectivity_event_received(struct nfc_hci_dev *hdev, u8 host,
transaction = (struct nfc_evt_transaction *)devm_kzalloc(dev,
skb->len - 2, GFP_KERNEL);
+ if (!transaction)
+ return -ENOMEM;
transaction->aid_len = skb->data[1];
memcpy(transaction->aid, &skb->data[2],
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next] [net-next] mlx4: avoid large stack usage in mlx4_init_hca()
From: Saeed Mahameed @ 2019-07-23 22:09 UTC (permalink / raw)
To: davem@davemloft.net, Tariq Toukan, arnd@arndb.de
Cc: linux-rdma@vger.kernel.org, jackm@dev.mellanox.co.il, Erez Alfasi,
Moshe Shemesh, Eli Cohen, linux-kernel@vger.kernel.org,
Jiri Pirko, netdev@vger.kernel.org,
clang-built-linux@googlegroups.com
In-Reply-To: <20190722150204.1157315-1-arnd@arndb.de>
On Mon, 2019-07-22 at 17:01 +0200, Arnd Bergmann wrote:
> The mlx4_dev_cap and mlx4_init_hca_param are really too large
> to be put on the kernel stack, as shown by this clang warning:
>
> drivers/net/ethernet/mellanox/mlx4/main.c:3304:12: error: stack frame
> size of 1088 bytes in function 'mlx4_load_one' [-Werror,-Wframe-
> larger-than=]
>
> With gcc, the problem is the same, but it does not warn because
> it does not inline this function, and therefore stays just below
> the warning limit, while clang is just above it.
>
> Use kzalloc for dynamic allocation instead of putting them
> on stack. This gets the combined stack frame down to 424 bytes.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 66 +++++++++++++------
> ----
> 1 file changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c
> b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 1f6e16d5ea6b..07c204bd3fc4 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -2292,23 +2292,31 @@ static int mlx4_init_fw(struct mlx4_dev *dev)
> static int mlx4_init_hca(struct mlx4_dev *dev)
> {
> struct mlx4_priv *priv = mlx4_priv(dev);
> + struct mlx4_init_hca_param *init_hca = NULL;
> + struct mlx4_dev_cap *dev_cap = NULL;
> struct mlx4_adapter adapter;
> - struct mlx4_dev_cap dev_cap;
> struct mlx4_profile profile;
> - struct mlx4_init_hca_param init_hca;
> u64 icm_size;
> struct mlx4_config_dev_params params;
> int err;
>
> if (!mlx4_is_slave(dev)) {
> - err = mlx4_dev_cap(dev, &dev_cap);
> + dev_cap = kzalloc(sizeof(*dev_cap), GFP_KERNEL);
> + init_hca = kzalloc(sizeof(*init_hca), GFP_KERNEL);
> +
> + if (!dev_cap || !init_hca) {
> + err = -ENOMEM;
> + goto out_free;
> + }
> +
> + err = mlx4_dev_cap(dev, dev_cap);
> if (err) {
> mlx4_err(dev, "QUERY_DEV_CAP command failed,
> aborting\n");
> - return err;
> + goto out_free;
> }
>
> - choose_steering_mode(dev, &dev_cap);
> - choose_tunnel_offload_mode(dev, &dev_cap);
> + choose_steering_mode(dev, dev_cap);
> + choose_tunnel_offload_mode(dev, dev_cap);
>
> if (dev->caps.dmfs_high_steer_mode ==
> MLX4_STEERING_DMFS_A0_STATIC &&
> mlx4_is_master(dev))
> @@ -2331,48 +2339,48 @@ static int mlx4_init_hca(struct mlx4_dev
> *dev)
> MLX4_STEERING_MODE_DEVICE_MANAGED)
> profile.num_mcg = MLX4_FS_NUM_MCG;
>
> - icm_size = mlx4_make_profile(dev, &profile, &dev_cap,
> - &init_hca);
> + icm_size = mlx4_make_profile(dev, &profile, dev_cap,
> + init_hca);
> if ((long long) icm_size < 0) {
> err = icm_size;
> - return err;
> + goto out_free;
> }
>
> dev->caps.max_fmr_maps = (1 << (32 - ilog2(dev-
> >caps.num_mpts))) - 1;
>
> if (enable_4k_uar || !dev->persist->num_vfs) {
> - init_hca.log_uar_sz = ilog2(dev->caps.num_uars)
> +
> + init_hca->log_uar_sz = ilog2(dev-
> >caps.num_uars) +
> PAGE_SHIFT -
> DEFAULT_UAR_PAGE_SHIFT;
> - init_hca.uar_page_sz = DEFAULT_UAR_PAGE_SHIFT -
> 12;
> + init_hca->uar_page_sz = DEFAULT_UAR_PAGE_SHIFT
> - 12;
> } else {
> - init_hca.log_uar_sz = ilog2(dev-
> >caps.num_uars);
> - init_hca.uar_page_sz = PAGE_SHIFT - 12;
> + init_hca->log_uar_sz = ilog2(dev-
> >caps.num_uars);
> + init_hca->uar_page_sz = PAGE_SHIFT - 12;
> }
>
> - init_hca.mw_enabled = 0;
> + init_hca->mw_enabled = 0;
> if (dev->caps.flags & MLX4_DEV_CAP_FLAG_MEM_WINDOW ||
> dev->caps.bmme_flags & MLX4_BMME_FLAG_TYPE_2_WIN)
> - init_hca.mw_enabled = INIT_HCA_TPT_MW_ENABLE;
> + init_hca->mw_enabled = INIT_HCA_TPT_MW_ENABLE;
>
> - err = mlx4_init_icm(dev, &dev_cap, &init_hca,
> icm_size);
> + err = mlx4_init_icm(dev, dev_cap, init_hca, icm_size);
> if (err)
> - return err;
> + goto out_free;
>
> - err = mlx4_INIT_HCA(dev, &init_hca);
> + err = mlx4_INIT_HCA(dev, init_hca);
> if (err) {
> mlx4_err(dev, "INIT_HCA command failed,
> aborting\n");
> goto err_free_icm;
> }
>
> - if (dev_cap.flags2 & MLX4_DEV_CAP_FLAG2_SYS_EQS) {
> - err = mlx4_query_func(dev, &dev_cap);
> + if (dev_cap->flags2 & MLX4_DEV_CAP_FLAG2_SYS_EQS) {
> + err = mlx4_query_func(dev, dev_cap);
> if (err < 0) {
> mlx4_err(dev, "QUERY_FUNC command
> failed, aborting.\n");
> goto err_close;
> } else if (err & MLX4_QUERY_FUNC_NUM_SYS_EQS) {
> - dev->caps.num_eqs = dev_cap.max_eqs;
> - dev->caps.reserved_eqs =
> dev_cap.reserved_eqs;
> - dev->caps.reserved_uars =
> dev_cap.reserved_uars;
> + dev->caps.num_eqs = dev_cap->max_eqs;
> + dev->caps.reserved_eqs = dev_cap-
> >reserved_eqs;
> + dev->caps.reserved_uars = dev_cap-
> >reserved_uars;
> }
> }
>
> @@ -2381,14 +2389,13 @@ static int mlx4_init_hca(struct mlx4_dev
> *dev)
> * read HCA frequency by QUERY_HCA command
> */
> if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_TS) {
> - memset(&init_hca, 0, sizeof(init_hca));
> - err = mlx4_QUERY_HCA(dev, &init_hca);
> + err = mlx4_QUERY_HCA(dev, init_hca);
> if (err) {
> mlx4_err(dev, "QUERY_HCA command
> failed, disable timestamp\n");
> dev->caps.flags2 &=
> ~MLX4_DEV_CAP_FLAG2_TS;
> } else {
> dev->caps.hca_core_clock =
> - init_hca.hca_core_clock;
> + init_hca->hca_core_clock;
> }
>
> /* In case we got HCA frequency 0 - disable
> timestamping
> @@ -2464,7 +2471,8 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
> priv->eq_table.inta_pin = adapter.inta_pin;
> memcpy(dev->board_id, adapter.board_id, sizeof(dev->board_id));
>
> - return 0;
> + err = 0;
> + goto out_free;
>
> unmap_bf:
> unmap_internal_clock(dev);
> @@ -2483,6 +2491,10 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
> if (!mlx4_is_slave(dev))
> mlx4_free_icms(dev);
>
> +out_free:
> + kfree(dev_cap);
> + kfree(init_hca);
> +
> return err;
> }
>
^ permalink raw reply
* Re: [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace
From: Alexei Starovoitov @ 2019-07-23 22:11 UTC (permalink / raw)
To: Joel Fernandes
Cc: LKML, bpf, Daniel Borkmann, Network Development, Steven Rostedt,
kernel-team
In-Reply-To: <20190718025143.GB153617@google.com>
On Wed, Jul 17, 2019 at 10:51:43PM -0400, Joel Fernandes wrote:
> Hi Alexei,
>
> On Wed, Jul 17, 2019 at 02:40:42PM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 17, 2019 at 6:01 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > I trimmed cc. some emails were bouncing.
>
> Ok, thanks.
>
> > > > I think allowing one tracepoint and disallowing another is pointless
> > > > from security point of view. Tracing bpf program can do bpf_probe_read
> > > > of anything.
> > >
> > > I think the assumption here is the user controls the program instructions at
> > > runtime, but that's not the case. The BPF program we are loading is not
> > > dynamically generated, it is built at build time and it is loaded from a
> > > secure verified partition, so even though it can do bpf_probe_read, it is
> > > still not something that the user can change.
> >
> > so you're saying that by having a set of signed bpf programs which
> > instructions are known to be non-malicious and allowed set of tracepoints
> > to attach via selinux whitelist, such setup will be safe?
> > Have you considered how mix and match will behave?
>
> Do you mean the effect of mixing tracepoints and programs? I have not
> considered this. I am Ok with further enforcing of this (only certain
> tracepoints can be attached to certain programs) if needed. What do
> you think? We could have a new bpf(2) syscall attribute specify which
> tracepoint is expected, or similar.
>
> I wanted to walk you through our 2 usecases we are working on:
thanks for sharing the use case details. Appreciate it.
> 1. timeinstate: By hooking 2 programs onto sched_switch and cpu_frequency
> tracepoints, we are able to collect CPU power per-UID (specific app). Connor
> O'Brien is working on that.
>
> 2. inode to file path mapping: By hooking onto VFS tracepoints we are adding to
> the android kernels, we can collect data when the kernel resolves a file path
> to a inode/device number. A BPF map stores the inode/dev number (key) and the
> path (value). We have usecases where we need a high speed lookup of this
> without having to scan all the files in the filesystem.
Can you share the link to vfs tracepoints you're adding?
Sounds like you're not going to attempt to upstream them knowing
Al's stance towards them?
May be there is a way we can do the feature you need, but w/o tracepoints?
> For the first usecase, the BPF program will be loaded and attached to the
> scheduler and cpufreq tracepoints at boot time and will stay attached
> forever. This is why I was saying having a daemon to stay alive all the time
> is pointless. However, if since you are completely against using tracefs
> which it sounds like, then we can do a daemon that is always alive.
As I said earlier this use case can be solved by pinning raw_tp object
into bpffs. Such patches are welcomed.
> For the second usecase, the program attach is needed on-demand unlike the
> first usecase, and then after the usecase completes, it is detached to avoid
> overhead.
>
> For the second usecase, privacy is important and we want the data to not be
> available to any process. So we want to make sure only selected processes can
> attach to that tracepoint. This is the reason why I was doing working on
> these patches which use the tracefs as well, since we get that level of
> control.
It's hard to recommend anything w/o seeing the actual tracepoints you're adding
to vfs and type of data bpf program extracts from there.
Sounds like it's some sort of cache of inode->file name ?
If so, why is it privacy related?
^ permalink raw reply
* [PATCH] st_nci_hci_connectivity_event_received: null check the allocation
From: Navid Emamdoost @ 2019-07-23 22:11 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, secalert, Navid Emamdoost, Kate Stewart,
Greg Kroah-Hartman, Thomas Gleixner, Allison Randal, netdev,
linux-kernel
devm_kzalloc may fail and return NULL. So the null check is needed.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/nfc/st-nci/se.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nfc/st-nci/se.c b/drivers/nfc/st-nci/se.c
index c3e10b6ab3a4..f25f1ec5f9e9 100644
--- a/drivers/nfc/st-nci/se.c
+++ b/drivers/nfc/st-nci/se.c
@@ -333,6 +333,8 @@ static int st_nci_hci_connectivity_event_received(struct nci_dev *ndev,
transaction = (struct nfc_evt_transaction *)devm_kzalloc(dev,
skb->len - 2, GFP_KERNEL);
+ if (!transaction)
+ return -ENOMEM;
transaction->aid_len = skb->data[1];
memcpy(transaction->aid, &skb->data[2], transaction->aid_len);
--
2.17.1
^ permalink raw reply related
* Re: memory leak in rds_send_probe
From: syzbot @ 2019-07-23 22:17 UTC (permalink / raw)
To: akpm, catalin.marinas, davem, dvyukov, jack, kirill.shutemov,
koct9i, linux-kernel, linux-mm, linux-rdma, neilb, netdev,
rds-devel, ross.zwisler, santosh.shilimkar, syzkaller-bugs,
torvalds, willy
In-Reply-To: <000000000000ad1dfe058e5b89ab@google.com>
syzbot has bisected this bug to:
commit af49a63e101eb62376cc1d6bd25b97eb8c691d54
Author: Matthew Wilcox <willy@linux.intel.com>
Date: Sat May 21 00:03:33 2016 +0000
radix-tree: change naming conventions in radix_tree_shrink
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=176528c8600000
start commit: c6dd78fc Merge branch 'x86-urgent-for-linus' of git://git...
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=14e528c8600000
console output: https://syzkaller.appspot.com/x/log.txt?x=10e528c8600000
kernel config: https://syzkaller.appspot.com/x/.config?x=8de7d700ea5ac607
dashboard link: https://syzkaller.appspot.com/bug?extid=5134cdf021c4ed5aaa5f
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=145df0c8600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=170001f4600000
Reported-by: syzbot+5134cdf021c4ed5aaa5f@syzkaller.appspotmail.com
Fixes: af49a63e101e ("radix-tree: change naming conventions in
radix_tree_shrink")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH bpf] libbpf: fix using uninitialized ioctl results
From: Alexei Starovoitov @ 2019-07-23 22:18 UTC (permalink / raw)
To: Ilya Maximets
Cc: Network Development, bpf, David S. Miller, Magnus Karlsson,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <20190723120810.28801-1-i.maximets@samsung.com>
On Tue, Jul 23, 2019 at 5:08 AM Ilya Maximets <i.maximets@samsung.com> wrote:
>
> 'channels.max_combined' initialized only on ioctl success and
> errno is only valid on ioctl failure.
>
> The code doesn't produce any runtime issues, but makes memory
> sanitizers angry:
>
> Conditional jump or move depends on uninitialised value(s)
> at 0x55C056F: xsk_get_max_queues (xsk.c:336)
> by 0x55C05B2: xsk_create_bpf_maps (xsk.c:354)
> by 0x55C089F: xsk_setup_xdp_prog (xsk.c:447)
> by 0x55C0E57: xsk_socket__create (xsk.c:601)
> Uninitialised value was created by a stack allocation
> at 0x55C04CD: xsk_get_max_queues (xsk.c:318)
>
> Additionally fixed warning on uninitialized bytes in ioctl arguments:
>
> Syscall param ioctl(SIOCETHTOOL) points to uninitialised byte(s)
> at 0x648D45B: ioctl (in /usr/lib64/libc-2.28.so)
> by 0x55C0546: xsk_get_max_queues (xsk.c:330)
> by 0x55C05B2: xsk_create_bpf_maps (xsk.c:354)
> by 0x55C089F: xsk_setup_xdp_prog (xsk.c:447)
> by 0x55C0E57: xsk_socket__create (xsk.c:601)
> Address 0x1ffefff378 is on thread 1's stack
> in frame #1, created by xsk_get_max_queues (xsk.c:318)
> Uninitialised value was created by a stack allocation
> at 0x55C04CD: xsk_get_max_queues (xsk.c:318)
>
> CC: Magnus Karlsson <magnus.karlsson@intel.com>
> Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> tools/lib/bpf/xsk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 5007b5d4fd2c..c4f912dc30f9 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -317,7 +317,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
>
> static int xsk_get_max_queues(struct xsk_socket *xsk)
> {
> - struct ethtool_channels channels;
> + struct ethtool_channels channels = { .cmd = ETHTOOL_GCHANNELS };
> struct ifreq ifr;
> int fd, err, ret;
>
> @@ -325,7 +325,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> if (fd < 0)
> return -errno;
>
> - channels.cmd = ETHTOOL_GCHANNELS;
> + memset(&ifr, 0, sizeof(ifr));
I refactored this bit into
struct ifreq ifr = {};
based on Andrii suggestion and pushed to bpf tree.
Thanks
> ifr.ifr_data = (void *)&channels;
> strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
> ifr.ifr_name[IFNAMSIZ - 1] = '\0';
> @@ -335,7 +335,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> goto out;
> }
>
> - if (channels.max_combined == 0 || errno == EOPNOTSUPP)
> + if (err || channels.max_combined == 0)
> /* If the device says it has no channels, then all traffic
> * is sent to a single stream, so max queues = 1.
> */
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] mt76_init_sband_2g: null check the allocation
From: Navid Emamdoost @ 2019-07-23 22:19 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, secalert, Navid Emamdoost,
Jakub Kicinski, Kalle Valo, David S. Miller, Matthias Brugger,
linux-wireless, netdev, linux-arm-kernel, linux-mediatek,
linux-kernel
devm_kzalloc may fail and return NULL. So the null check is needed.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/wireless/mediatek/mt7601u/init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c
index 9bfac9f1d47f..cada48800928 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -557,6 +557,9 @@ mt76_init_sband_2g(struct mt7601u_dev *dev)
{
dev->sband_2g = devm_kzalloc(dev->dev, sizeof(*dev->sband_2g),
GFP_KERNEL);
+ if (!dev->sband_2g)
+ return -ENOMEM;
+
dev->hw->wiphy->bands[NL80211_BAND_2GHZ] = dev->sband_2g;
WARN_ON(dev->ee->reg.start - 1 + dev->ee->reg.num >
--
2.17.1
^ permalink raw reply related
* Re: memory leak in rds_send_probe
From: Andrew Morton @ 2019-07-23 22:23 UTC (permalink / raw)
To: syzbot
Cc: catalin.marinas, davem, dvyukov, jack, kirill.shutemov, koct9i,
linux-kernel, linux-mm, linux-rdma, neilb, netdev, rds-devel,
ross.zwisler, santosh.shilimkar, syzkaller-bugs, torvalds, willy
In-Reply-To: <00000000000034c84a058e608d45@google.com>
On Tue, 23 Jul 2019 15:17:00 -0700 syzbot <syzbot+5134cdf021c4ed5aaa5f@syzkaller.appspotmail.com> wrote:
> syzbot has bisected this bug to:
>
> commit af49a63e101eb62376cc1d6bd25b97eb8c691d54
> Author: Matthew Wilcox <willy@linux.intel.com>
> Date: Sat May 21 00:03:33 2016 +0000
>
> radix-tree: change naming conventions in radix_tree_shrink
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=176528c8600000
> start commit: c6dd78fc Merge branch 'x86-urgent-for-linus' of git://git...
> git tree: upstream
> final crash: https://syzkaller.appspot.com/x/report.txt?x=14e528c8600000
> console output: https://syzkaller.appspot.com/x/log.txt?x=10e528c8600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=8de7d700ea5ac607
> dashboard link: https://syzkaller.appspot.com/bug?extid=5134cdf021c4ed5aaa5f
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=145df0c8600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=170001f4600000
>
> Reported-by: syzbot+5134cdf021c4ed5aaa5f@syzkaller.appspotmail.com
> Fixes: af49a63e101e ("radix-tree: change naming conventions in
> radix_tree_shrink")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
That's rather hard to believe. af49a63e101eb6237 simply renames a
couple of local variables.
^ permalink raw reply
* Re: [PATCH net-next 1/3] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Andrew Lunn @ 2019-07-23 22:24 UTC (permalink / raw)
To: Claudiu Manoil
Cc: David S . Miller, devicetree, netdev, alexandru.marginean,
linux-kernel, Li Yang, Rob Herring, linux-arm-kernel
In-Reply-To: <1563894955-545-2-git-send-email-claudiu.manoil@nxp.com>
> + bus = mdiobus_alloc_size(sizeof(u32 *));
> + if (!bus)
> + return -ENOMEM;
> +
> + bus->priv = pci_iomap_range(pdev, 0, ENETC_MDIO_REG_OFFSET, 0);
This got me confused for a while. You allocate space for a u32
pointer. bus->priv will point to this space. However, you are not
using this space, you {ab}use the pointer to directly hold the return
from pci_iomap_range(). This works, but sparse is probably unhappy,
and you are wasting the space the u32 pointer takes.
Andrew
^ permalink raw reply
* [PATCH] stmmac_dt_phy: null check the allocation
From: Navid Emamdoost @ 2019-07-23 22:28 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, secalert, Navid Emamdoost,
Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel,
linux-kernel
devm_kzalloc may fail and return NULL. So the null check is needed.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 73fc2524372e..392f8d9539c1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -342,10 +342,13 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
mdio = true;
}
- if (mdio)
+ if (mdio) {
plat->mdio_bus_data =
devm_kzalloc(dev, sizeof(struct stmmac_mdio_bus_data),
GFP_KERNEL);
+ if (!plat->mdio_bus_data)
+ return -ENOMEM;
+ }
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 4/7] net: Reorder the contents of skb_frag_t
From: Saeed Mahameed @ 2019-07-23 22:29 UTC (permalink / raw)
To: willy@infradead.org, davem@davemloft.net
Cc: hch@lst.de, netdev@vger.kernel.org
In-Reply-To: <20190712134345.19767-5-willy@infradead.org>
On Fri, 2019-07-12 at 06:43 -0700, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Match the layout of bio_vec.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> include/linux/skbuff.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 7910935410e6..b9dc8b4f24b1 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -314,8 +314,8 @@ struct skb_frag_struct {
> struct {
> struct page *p;
> } page;
> - __u32 page_offset;
> __u32 size;
> + __u32 page_offset;
> };
>
Why do you need this patch? this struct is going to be removed
downstream eventually ..
> /**
^ permalink raw reply
* Re: [PATCH v3 6/7] net: Rename skb_frag_t size to bv_len
From: Saeed Mahameed @ 2019-07-23 22:33 UTC (permalink / raw)
To: willy@infradead.org, davem@davemloft.net
Cc: hch@lst.de, netdev@vger.kernel.org
In-Reply-To: <20190712134345.19767-7-willy@infradead.org>
On Fri, 2019-07-12 at 06:43 -0700, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Improved compatibility with bvec
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> include/linux/skbuff.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 8076e2ba8349..e849e411d1f3 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -312,7 +312,7 @@ typedef struct skb_frag_struct skb_frag_t;
>
> struct skb_frag_struct {
> struct page *bv_page;
> - __u32 size;
> + unsigned int bv_len;
> __u32 page_offset;
Why do you keep page_offset name and type as is ? it will make the last
patch much cleaner if you change it to "unsigned int bv_offset".
unsigned int and __u32 on both 32bit and 64bit are the same aren't they
?
> };
>
> @@ -322,7 +322,7 @@ struct skb_frag_struct {
> */
> static inline unsigned int skb_frag_size(const skb_frag_t *frag)
> {
> - return frag->size;
> + return frag->bv_len;
> }
>
> /**
> @@ -332,7 +332,7 @@ static inline unsigned int skb_frag_size(const
> skb_frag_t *frag)
> */
> static inline void skb_frag_size_set(skb_frag_t *frag, unsigned int
> size)
> {
> - frag->size = size;
> + frag->bv_len = size;
> }
>
> /**
> @@ -342,7 +342,7 @@ static inline void skb_frag_size_set(skb_frag_t
> *frag, unsigned int size)
> */
> static inline void skb_frag_size_add(skb_frag_t *frag, int delta)
> {
> - frag->size += delta;
> + frag->bv_len += delta;
> }
>
> /**
> @@ -352,7 +352,7 @@ static inline void skb_frag_size_add(skb_frag_t
> *frag, int delta)
> */
> static inline void skb_frag_size_sub(skb_frag_t *frag, int delta)
> {
> - frag->size -= delta;
> + frag->bv_len -= delta;
> }
>
> /**
^ permalink raw reply
* Re: [PATCH v4 net-next 02/19] ionic: Add hardware init and device commands
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.141833.384334163321137202.davem@davemloft.net>
On 7/23/19 2:18 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:06 -0700
>
>> +void ionic_init_devinfo(struct ionic_dev *idev)
>> +{
>> + idev->dev_info.asic_type = ioread8(&idev->dev_info_regs->asic_type);
>> + idev->dev_info.asic_rev = ioread8(&idev->dev_info_regs->asic_rev);
>> +
>> + memcpy_fromio(idev->dev_info.fw_version,
>> + idev->dev_info_regs->fw_version,
>> + IONIC_DEVINFO_FWVERS_BUFLEN);
>> +
>> + memcpy_fromio(idev->dev_info.serial_num,
>> + idev->dev_info_regs->serial_num,
>> + IONIC_DEVINFO_SERIAL_BUFLEN);
> ...
>> + sig = ioread32(&idev->dev_info_regs->signature);
> I think if you are going to use the io{read,write}{8,16,32,64}()
> interfaces then you should use io{read,write}{8,16,32,64}_rep()
> instead of memcpy_{to,from}io().
>
Sure.
sln
^ permalink raw reply
* Re: [PATCH v4 net-next 05/19] ionic: Add interrupts and doorbells
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.142448.414859031558093111.davem@davemloft.net>
On 7/23/19 2:24 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:09 -0700
>
>> The ionic interrupt model is based on interrupt control blocks
>> accessed through the PCI BAR. Doorbell registers are used by
>> the driver to signal to the NIC that requests are waiting on
>> the message queues. Interrupts are used by the NIC to signal
>> to the driver that answers are waiting on the completion queues.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> After applying this patch we get a warning:
>
> drivers/net/ethernet/pensando/ionic/ionic_lif.c:33:13: warning: ‘ionic_intr_free’ defined but not used [-Wunused-function]
> static void ionic_intr_free(struct lif *lif, int index)
> ^~~~~~~~~~~~~~~
> drivers/net/ethernet/pensando/ionic/ionic_lif.c:15:12: warning: ‘ionic_intr_alloc’ defined but not used [-Wunused-function]
> static int ionic_intr_alloc(struct lif *lif, struct intr *intr)
> ^~~~~~~~~~~~~~~~
Yes, and they get used in the next patch. This is so I can keep similar
bits of functionality together in one patch and to keep the next patch a
little smaller.
> Also:
>
>> + lif->dbid_inuse = kzalloc(BITS_TO_LONGS(lif->dbid_count) * sizeof(long),
>> + GFP_KERNEL);
> You can use bitmap_alloc() and friends from linux/bitmap.h for this kind of stuff.
Sure.
sln
^ permalink raw reply
* Re: [PATCH v4 net-next 06/19] ionic: Add basic adminq support
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.142738.638894843366352833.davem@davemloft.net>
On 7/23/19 2:27 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:10 -0700
>
>> +struct queue {
> ...
>> +struct cq {
> ...
>> +struct napi_stats {
> ...
>> +struct q_stats {
> ...
>> +struct qcq {
> Using names like these and "dev_queue" are just asking for conflicts with the
> global datastructure namespace both now and in the future.
>
> Please put ionic_ or similar as a prefix to these data structure names.
>
> Thank you.
Sure
sln
^ permalink raw reply
* Re: [PATCH v4 net-next 11/19] ionic: Add Rx filter and rx_mode ndo support
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.143326.197667027838462669.davem@davemloft.net>
On 7/23/19 2:33 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:15 -0700
>
>> + if (in_interrupt()) {
>> + work = kzalloc(sizeof(*work), GFP_ATOMIC);
>> + if (!work) {
>> + netdev_err(lif->netdev, "%s OOM\n", __func__);
>> + return -ENOMEM;
>> + }
>> + work->type = add ? DW_TYPE_RX_ADDR_ADD : DW_TYPE_RX_ADDR_DEL;
>> + memcpy(work->addr, addr, ETH_ALEN);
>> + netdev_dbg(lif->netdev, "deferred: rx_filter %s %pM\n",
>> + add ? "add" : "del", addr);
>> + ionic_lif_deferred_enqueue(&lif->deferred, work);
>> + } else {
>> + netdev_dbg(lif->netdev, "rx_filter %s %pM\n",
>> + add ? "add" : "del", addr);
>> + if (add)
>> + return ionic_lif_addr_add(lif, addr);
>> + else
>> + return ionic_lif_addr_del(lif, addr);
>> + }
> I don't know about this.
>
> Generally interface address changes are expected to be synchronous.
Yeah, this bothers me a bit as well, but the address change calls come
in under spin_lock_bh(), and I'm reluctant to make an AdminQ call under
the _bh that could block for a few seconds.
sln
^ permalink raw reply
* Re: [PATCH v4 net-next 13/19] ionic: Add initial ethtool support
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.143507.1690183028498872104.davem@davemloft.net>
On 7/23/19 2:35 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:17 -0700
>
>> +static int ionic_get_link_ksettings(struct net_device *netdev,
>> + struct ethtool_link_ksettings *ks)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + int copper_seen = 0;
> Reverse christmas tree ordering here please.
Sure. Most of these are because I wanted to do the one assignment
first, from which the others are based. I'll rework these to add the
init lines after the declarations. Same in the other files.
sln
>> +static int ionic_set_link_ksettings(struct net_device *netdev,
>> + const struct ethtool_link_ksettings *ks)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic *ionic = lif->ionic;
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + u8 fec_type = PORT_FEC_TYPE_NONE;
>> + u32 req_rs, req_fc;
>> + int err = 0;
> Likewise.
>> +static void ionic_get_pauseparam(struct net_device *netdev,
>> + struct ethtool_pauseparam *pause)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + uint8_t pause_type = idev->port_info->config.pause_type;
> Likewise.
>
>> +static int ionic_set_pauseparam(struct net_device *netdev,
>> + struct ethtool_pauseparam *pause)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic *ionic = lif->ionic;
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + u32 requested_pause;
>> + int err;
> Likewise.
>> +static int ionic_get_module_info(struct net_device *netdev,
>> + struct ethtool_modinfo *modinfo)
>> +
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + struct xcvr_status *xcvr;
> Likewise.
>
>> +static int ionic_get_module_eeprom(struct net_device *netdev,
>> + struct ethtool_eeprom *ee,
>> + u8 *data)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> + struct ionic_dev *idev = &lif->ionic->idev;
>> + struct xcvr_status *xcvr;
>> + u32 len;
> Likewise.
^ permalink raw reply
* Re: [PATCH v4 net-next 19/19] ionic: Add basic devlink interface
From: Shannon Nelson @ 2019-07-23 22:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.144055.138556918172139772.davem@davemloft.net>
On 7/23/19 2:40 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Mon, 22 Jul 2019 14:40:23 -0700
>
>> +struct ionic *ionic_devlink_alloc(struct device *dev)
>> +{
>> + struct devlink *dl;
>> + struct ionic *ionic;
> Reverse christmas tree please.
Yep, I missed this one.
Thanks for your review time.
sln
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox