* Re: [PATCH] mt76_init_sband_2g: null check the allocation
From: Jakub Kicinski @ 2019-07-24 17:15 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, kjlu, smccaman, secalert, Kalle Valo, David S. Miller,
Matthias Brugger, linux-wireless, netdev, linux-arm-kernel,
linux-mediatek, linux-kernel
In-Reply-To: <20190723221954.9233-1-navid.emamdoost@gmail.com>
On Tue, 23 Jul 2019 17:19:54 -0500, Navid Emamdoost wrote:
> devm_kzalloc may fail and return NULL. So the null check is needed.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Acked-by: Jakub Kicinski <kubakici@wp.pl>
Thanks!
^ permalink raw reply
* Re: [PATCH v4 net-next 02/19] ionic: Add hardware init and device commands
From: Shannon Nelson @ 2019-07-24 17:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190723.160538.2065000079755912945.davem@davemloft.net>
On 7/23/19 4:05 PM, David Miller wrote:
> From: Shannon Nelson <snelson@pensando.io>
> Date: Tue, 23 Jul 2019 15:50:22 -0700
>
>> 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.
> Note, I could be wrong. Please test.
>
> I think the operation of the two things might be different.
Yes, they are different things, the iowrite*_rep() functions write each
value from a buffer all to the same single address, rather than copy a
buffer into another buffer, and the ioread*_rep() functions repeatedly
read from the same address to fill a buffer.
For example, the iowrite32_rep() boils down to this:
do {
__raw_writel(*buf++, addr);
} while (--count);
Not quite what I need.
sln
^ permalink raw reply
* Re: [PATCH v2] net: dsa: qca8k: enable port flow control
From: Andrew Lunn @ 2019-07-24 17:50 UTC (permalink / raw)
To: xiaofeis
Cc: davem, vkoul, netdev, linux-arm-msm, bjorn.andersson,
vivien.didelot, f.fainelli, niklas.cassel, xiazha
In-Reply-To: <1563944576-62844-1-git-send-email-xiaofeis@codeaurora.org>
On Wed, Jul 24, 2019 at 01:02:56PM +0800, xiaofeis wrote:
> Set phy device advertising to enable MAC flow control.
Hi Xiaofeis
It seems like you are still using the wrong, and old tree. Please
rebase on net-next/master.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v3 4/7] net: Reorder the contents of skb_frag_t
From: Saeed Mahameed @ 2019-07-24 17:55 UTC (permalink / raw)
To: willy@infradead.org
Cc: hch@lst.de, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20190724012828.GQ363@bombadil.infradead.org>
On Tue, 2019-07-23 at 18:28 -0700, Matthew Wilcox wrote:
> On Tue, Jul 23, 2019 at 10:29:06PM +0000, Saeed Mahameed wrote:
> > 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 ..
>
> If there's a performance regression, this is the perfect patch to
> include
> as part of the bisection. You'd think that this change could have no
> effect, but I've seen weirder things.
ok make sense !
^ permalink raw reply
* Re: [PATCH v3 6/7] net: Rename skb_frag_t size to bv_len
From: Saeed Mahameed @ 2019-07-24 17:58 UTC (permalink / raw)
To: willy@infradead.org
Cc: hch@lst.de, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20190724013055.GR363@bombadil.infradead.org>
On Tue, 2019-07-23 at 18:30 -0700, Matthew Wilcox wrote:
> On Tue, Jul 23, 2019 at 10:33:59PM +0000, Saeed Mahameed wrote:
> > > 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".
>
> We don't have an accessor for page_offset, so there are about 280
> occurrences of '>page_offset' in drivers/net/
>
I understand but why not chaining the type to "unsigned int" at least
to avoid confusion ?
> Feel free to be the hero who does that cleanup.
^ permalink raw reply
* [PATCH net-next] net/tls: add myself as a co-maintainer
From: Jakub Kicinski @ 2019-07-24 18:02 UTC (permalink / raw)
To: davem, alexei.starovoitov, daniel, john.fastabend, davejwatson,
borisp, aviadye
Cc: netdev, oss-drivers, simon.horman, Jakub Kicinski,
Alexei Starovoitov
I've been spending quite a bit of time fixing and
preventing bit rot in the core TLS code. TLS seems
to only be growing in importance, I'd like to help
ensuring the quality of our implementation.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Simon Horman <simon.horman@netronome.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e3c4b4..3ff2e6ab3cf4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11282,6 +11282,7 @@ M: Aviad Yehezkel <aviadye@mellanox.com>
M: Dave Watson <davejwatson@fb.com>
M: John Fastabend <john.fastabend@gmail.com>
M: Daniel Borkmann <daniel@iogearbox.net>
+M: Jakub Kicinski <jakub.kicinski@netronome.com>
L: netdev@vger.kernel.org
S: Maintained
F: net/tls/*
--
2.21.0
^ permalink raw reply related
* Re: [RFC] dt-bindings: net: phy: Add subnode for LED configuration
From: Andrew Lunn @ 2019-07-24 18:04 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190722223741.113347-1-mka@chromium.org>
On Mon, Jul 22, 2019 at 03:37:41PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be 'on' when a link with a certain speed
> is active, or to blink on RX/TX activity. For the configuration to
> be effective it needs to be supported by the hardware and the
> corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> This RFC is a follow up of the discussion on "[PATCH v2 6/7]
> dt-bindings: net: realtek: Add property to configure LED mode"
> (https://lore.kernel.org/patchwork/patch/1097185/).
>
> For now posting as RFC to get a basic agreement on the bindings
> before proceding with the implementation in phylib and a specific
> driver.
> ---
> Documentation/devicetree/bindings/net/phy.txt | 33 +++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index 9b9e5b1765dd..ad495d3abbbb 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -46,6 +46,25 @@ Optional Properties:
> Mark the corresponding energy efficient ethernet mode as broken and
> request the ethernet to stop advertising it.
>
> +- leds: A sub-node which is a container of only LED nodes. Each child
> + node represents a PHY LED.
> +
> + Required properties for LED child nodes:
> + - reg: The ID number of the LED, typically corresponds to a hardware ID.
> +
> + Optional properties for child nodes:
> + - label: The label for this LED. If omitted, the label is taken from the node
> + name (excluding the unit address). It has to uniquely identify a device,
> + i.e. no other LED class device can be assigned the same label.
Hi Matthias
I've thought about label a bit more.
> + label = "ethphy0:left:green";
We need to be careful with names here. systemd etc renames
interfaces. ethphy0 could in fact be connected to enp3s0, or eth0
might get renamed to eth1, etc. So i think we should avoid things like
ethphy0. Also, i'm not sure we actually need a label, at least not to
start with.Do we have any way to expose it to the user?
If we do ever make it part of the generic LED framework, we can then
use the label. At that point, i would probably combine the label with
the interface name in a dynamic way to avoid issues like this.
Andrew
^ permalink raw reply
* Re: Reminder: 99 open syzbot bugs in net subsystem
From: David Miller @ 2019-07-24 18:12 UTC (permalink / raw)
To: ebiggers
Cc: eric.dumazet, dvyukov, netdev, fw, i.maximets, edumazet, dsahern,
linux-kernel, syzkaller-bugs
In-Reply-To: <20190724163014.GC673@sol.localdomain>
From: Eric Biggers <ebiggers@kernel.org>
Date: Wed, 24 Jul 2019 09:30:14 -0700
> On Wed, Jul 24, 2019 at 08:39:05AM +0200, Eric Dumazet wrote:
>> Some of the bugs have been fixed already, before syzbot found them.
>>
>> Why force human to be gentle to bots and actually replying to them ?
>>
>> I usually simply wait that syzbot is finding the bug does not repro anymore,
>> but now if you send these emails, we will have even more pressure on us.
>>
>
> First, based on experience, I'd guess about 30-45 of these are still valid. 17
> were seen in mainline in the last week, but some others are valid too. The ones
> most likely to still be valid are at the beginning of the list. So let's try
> not use the presence of outdated bugs as an excuse not to fix current bugs.
So about half of the bugs we are to look at are already fixed and thus
noise, even as estimated by you.
I agree with Eric, these "reminders" are bad for the people you
actually want to work on fixing these bugs.
> Since the kernel community is basically in continuous bug bankruptcy and lots of
I don't like this hyperbole. Please present facts and information we
can actually use to improve the kernel development and bug fixing
process.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] Use dev_get_drvdata where possible
From: David Miller @ 2019-07-24 18:14 UTC (permalink / raw)
To: hslester96
Cc: klassert, jcliburn, chris.snook, rmody, michael.chan, siva.kallam,
prashant, GR-Linux-NIC-Dev, jeffrey.t.kirsher, cooldavid,
intel-wired-lan, netdev, linux-kernel
In-Reply-To: <20190724060512.23899-1-hslester96@gmail.com>
From: Chuhong Yuan <hslester96@gmail.com>
Date: Wed, 24 Jul 2019 14:05:12 +0800
> These patches use dev_get_drvdata instead of
> using to_pci_dev + pci_get_drvdata to make
> code simpler where possible.
>
> Changelog:
>
> v1 -> v2:
> - Change pci_set_drvdata to dev_set_drvdata
> to keep consistency.
Some of these patches were applied yesterday, weren't they?
Please take that into consideration when you repost, especially
when the maintainer(s) explicitly reply to your patches saying
"Applied." or similar like I always do.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 04/11] net: hns3: fix mis-counting IRQ vector numbers issue
From: Saeed Mahameed @ 2019-07-24 18:28 UTC (permalink / raw)
To: tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
liuyonglong@huawei.com
In-Reply-To: <1563938327-9865-5-git-send-email-tanhuazhong@huawei.com>
On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
>
> The num_msi_left means the vector numbers of NIC, but if the
> PF supported RoCE, it contains the vector numbers of NIC and
> RoCE(Not expected).
>
> This may cause interrupts lost in some case, because of the
> NIC module used the vector resources which belongs to RoCE.
>
> This patch corrects the value of num_msi_left to be equals to
> the vector numbers of NIC, and adjust the default tqp numbers
> according to the value of num_msi_left.
>
> Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine &
> Compatibility Layer Support")
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 5 ++++-
> drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 12
> ++++++++++--
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index 3c64d70..a59d13f 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -1470,13 +1470,16 @@ static int hclge_vport_setup(struct
> hclge_vport *vport, u16 num_tqps)
> {
> struct hnae3_handle *nic = &vport->nic;
> struct hclge_dev *hdev = vport->back;
> + u16 alloc_tqps;
> int ret;
>
> nic->pdev = hdev->pdev;
> nic->ae_algo = &ae_algo;
> nic->numa_node_mask = hdev->numa_node_mask;
>
> - ret = hclge_knic_setup(vport, num_tqps,
> + alloc_tqps = min_t(u16, hdev->roce_base_msix_offset - 1,
Why do you need the extra alloc_tqps ? just overwrite num_tqps, the
original value is not needed afterwards.
> num_tqps);
> +
> + ret = hclge_knic_setup(vport, alloc_tqps,
> hdev->num_tx_desc, hdev->num_rx_desc);
> if (ret)
> dev_err(&hdev->pdev->dev, "knic setup failed %d\n",
> ret);
>
^ permalink raw reply
* Re: [PATCH v3] drivers: net: xgene: Remove acpi_has_method() calls
From: David Miller @ 2019-07-24 18:30 UTC (permalink / raw)
To: skunberg.kelsey
Cc: iyappan, keyur, quan, netdev, linux-kernel, skhan,
linux-kernel-mentees
In-Reply-To: <20190724060659.105292-1-skunberg.kelsey@gmail.com>
From: Kelsey Skunberg <skunberg.kelsey@gmail.com>
Date: Wed, 24 Jul 2019 00:06:59 -0600
> acpi_evaluate_object will already return an error if the needed method
> does not exist. Remove unnecessary acpi_has_method() calls and check the
> returned acpi_status for failure instead.
>
> Signed-off-by: Kelsey Skunberg <skunberg.kelsey@gmail.com>
> ---
> Changes in v2:
> - Fixed white space warnings and errors
>
> Changes in v3:
> - Resolved build errors caused by missing bracket
Applied, will push out after build testing :)
^ permalink raw reply
* Re: [PATCH net-next 06/11] net: hns3: modify firmware version display format
From: Saeed Mahameed @ 2019-07-24 18:34 UTC (permalink / raw)
To: tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
moyufeng@huawei.com
In-Reply-To: <1563938327-9865-7-git-send-email-tanhuazhong@huawei.com>
On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> From: Yufeng Mo <moyufeng@huawei.com>
>
> This patch modifies firmware version display format in
> hclge(vf)_cmd_init() and hns3_get_drvinfo(). Also, adds
> some optimizations for firmware version display format.
>
> Signed-off-by: Yufeng Mo <moyufeng@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 9
> +++++++++
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 15
> +++++++++++++--
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 10
> +++++++++-
> drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c | 11
> +++++++++--
> 4 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> index 48c7b70..a4624db 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
> @@ -179,6 +179,15 @@ struct hnae3_vector_info {
> #define HNAE3_RING_GL_RX 0
> #define HNAE3_RING_GL_TX 1
>
> +#define HNAE3_FW_VERSION_BYTE3_SHIFT 24
> +#define HNAE3_FW_VERSION_BYTE3_MASK GENMASK(31, 24)
> +#define HNAE3_FW_VERSION_BYTE2_SHIFT 16
> +#define HNAE3_FW_VERSION_BYTE2_MASK GENMASK(23, 16)
> +#define HNAE3_FW_VERSION_BYTE1_SHIFT 8
> +#define HNAE3_FW_VERSION_BYTE1_MASK GENMASK(15, 8)
> +#define HNAE3_FW_VERSION_BYTE0_SHIFT 0
> +#define HNAE3_FW_VERSION_BYTE0_MASK GENMASK(7, 0)
> +
> struct hnae3_ring_chain_node {
> struct hnae3_ring_chain_node *next;
> u32 tqp_index;
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index 5bff98a..e71c92b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -527,6 +527,7 @@ static void hns3_get_drvinfo(struct net_device
> *netdev,
> {
> struct hns3_nic_priv *priv = netdev_priv(netdev);
> struct hnae3_handle *h = priv->ae_handle;
> + u32 fw_version;
>
> if (!h->ae_algo->ops->get_fw_version) {
> netdev_err(netdev, "could not get fw version!\n");
> @@ -545,8 +546,18 @@ static void hns3_get_drvinfo(struct net_device
> *netdev,
> sizeof(drvinfo->bus_info));
> drvinfo->bus_info[ETHTOOL_BUSINFO_LEN - 1] = '\0';
>
> - snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
> "0x%08x",
> - priv->ae_handle->ae_algo->ops->get_fw_version(h));
> + fw_version = priv->ae_handle->ae_algo->ops->get_fw_version(h);
> +
> + snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
> + "%lu.%lu.%lu.%lu",
> + hnae3_get_field(fw_version,
> HNAE3_FW_VERSION_BYTE3_MASK,
> + HNAE3_FW_VERSION_BYTE3_SHIFT),
> + hnae3_get_field(fw_version,
> HNAE3_FW_VERSION_BYTE2_MASK,
> + HNAE3_FW_VERSION_BYTE2_SHIFT),
> + hnae3_get_field(fw_version,
> HNAE3_FW_VERSION_BYTE1_MASK,
> + HNAE3_FW_VERSION_BYTE1_SHIFT),
> + hnae3_get_field(fw_version,
> HNAE3_FW_VERSION_BYTE0_MASK,
> + HNAE3_FW_VERSION_BYTE0_SHIFT));
> }
>
> static u32 hns3_get_link(struct net_device *netdev)
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> index 22f6acd..c2320bf 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> @@ -419,7 +419,15 @@ int hclge_cmd_init(struct hclge_dev *hdev)
> }
> hdev->fw_version = version;
>
> - dev_info(&hdev->pdev->dev, "The firmware version is %08x\n",
> version);
> + pr_info_once("The firmware version is %lu.%lu.%lu.%lu\n",
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE3_MASK,
> + HNAE3_FW_VERSION_BYTE3_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE2_MASK,
> + HNAE3_FW_VERSION_BYTE2_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE1_MASK,
> + HNAE3_FW_VERSION_BYTE1_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE0_MASK,
> + HNAE3_FW_VERSION_BYTE0_SHIFT));
>
Device name/string will not be printed now, what happens if i have
multiple devices ? at least print the device name as it was before
> return 0;
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
> index 652b796..004125b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
> @@ -405,8 +405,15 @@ int hclgevf_cmd_init(struct hclgevf_dev *hdev)
> }
> hdev->fw_version = version;
>
> - dev_info(&hdev->pdev->dev, "The firmware version is %08x\n",
> version);
> -
> + pr_info_once("The firmware version is %lu.%lu.%lu.%lu\n",
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE3_MASK,
> + HNAE3_FW_VERSION_BYTE3_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE2_MASK,
> + HNAE3_FW_VERSION_BYTE2_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE1_MASK,
> + HNAE3_FW_VERSION_BYTE1_SHIFT),
> + hnae3_get_field(version,
> HNAE3_FW_VERSION_BYTE0_MASK,
> + HNAE3_FW_VERSION_BYTE0_SHIFT));
> return 0;
>
Same.
^ permalink raw reply
* Re: Reminder: 99 open syzbot bugs in net subsystem
From: Eric Biggers @ 2019-07-24 18:37 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, dvyukov, netdev, fw, i.maximets, edumazet, dsahern,
linux-kernel, syzkaller-bugs
In-Reply-To: <20190724.111225.2257475150626507655.davem@davemloft.net>
On Wed, Jul 24, 2019 at 11:12:25AM -0700, David Miller wrote:
> From: Eric Biggers <ebiggers@kernel.org>
> Date: Wed, 24 Jul 2019 09:30:14 -0700
>
> > On Wed, Jul 24, 2019 at 08:39:05AM +0200, Eric Dumazet wrote:
> >> Some of the bugs have been fixed already, before syzbot found them.
> >>
> >> Why force human to be gentle to bots and actually replying to them ?
> >>
> >> I usually simply wait that syzbot is finding the bug does not repro anymore,
> >> but now if you send these emails, we will have even more pressure on us.
> >>
> >
> > First, based on experience, I'd guess about 30-45 of these are still valid. 17
> > were seen in mainline in the last week, but some others are valid too. The ones
> > most likely to still be valid are at the beginning of the list. So let's try
> > not use the presence of outdated bugs as an excuse not to fix current bugs.
>
> So about half of the bugs we are to look at are already fixed and thus
> noise, even as estimated by you.
>
> I agree with Eric, these "reminders" are bad for the people you
> actually want to work on fixing these bugs.
Well, the problem is that no one knows for sure which bugs are fixed and which
aren't. To be certain, a human needs to review each bug. A bot can only guess.
Note that the bugs in my reminders are already automatically prioritized by how
likely they are to still be valid, important, actionable. So one simply needs
to start at the beginning of the list if they want to focus on those types of
bugs. Isn't this helpful?
>
> > Since the kernel community is basically in continuous bug bankruptcy and lots of
>
> I don't like this hyperbole. Please present facts and information we
> can actually use to improve the kernel development and bug fixing
> process.
>
A huge number of valid open bugs are not being fixed, which is a fact. We can
argue about what words to use to describe this situation, but it doesn't change
the situation itself.
What is your proposed solution?
- Eric
^ permalink raw reply
* Re: pull-request: can-next 2019-07-24,pull-request: can-next 2019-07-24
From: David Miller @ 2019-07-24 18:39 UTC (permalink / raw)
To: mkl; +Cc: netdev, kernel, linux-can
In-Reply-To: <93540cba-184a-a9c5-f9d2-b1779a69a36f@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 24 Jul 2019 11:00:24 +0200
> this is a pull request for net-next/master consisting of 26 patches.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH net-next 00/10] Use dev_get_drvdata where possible
From: David Miller @ 2019-07-24 18:42 UTC (permalink / raw)
To: kvalo
Cc: hslester96, mlindner, stephen, jirislaby, mickflemm, mcgrof,
sgruszka, ath9k-devel, merez, johannes.berg, emmanuel.grumbach,
luciano.coelho, amitkarwar, nishants, gbhat, huxinming820,
imitsyanko, avinashp, smatyukevich, pkshih, linuxwifi,
linux-net-drivers, ecree, mhabets, netdev, wil6210,
linux-wireless, linux-kernel
In-Reply-To: <87zhl3zlu1.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Wed, 24 Jul 2019 14:57:42 +0300
> Do note that wireless patches go to wireless-drivers-next, not net-next.
> But I assume Dave will ignore patches 5-10 and I can take them.
Yes, that is what I plan to do.
^ permalink raw reply
* Re: [PATCH net-next 1/4] sctp: check addr_size with sa_family_t size in __sctp_setsockopt_connectx
From: Neil Horman @ 2019-07-24 18:44 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: Xin Long, network dev, linux-sctp, davem
In-Reply-To: <20190724124907.GA8640@localhost.localdomain>
On Wed, Jul 24, 2019 at 09:49:07AM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Jul 24, 2019 at 09:36:50AM -0300, Marcelo Ricardo Leitner wrote:
> > On Wed, Jul 24, 2019 at 07:22:35AM -0400, Neil Horman wrote:
> > > On Wed, Jul 24, 2019 at 03:21:12PM +0800, Xin Long wrote:
> > > > On Tue, Jul 23, 2019 at 11:25 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > On Tue, Jul 23, 2019 at 01:37:57AM +0800, Xin Long wrote:
> > > > > > Now __sctp_connect() is called by __sctp_setsockopt_connectx() and
> > > > > > sctp_inet_connect(), the latter has done addr_size check with size
> > > > > > of sa_family_t.
> > > > > >
> > > > > > In the next patch to clean up __sctp_connect(), we will remove
> > > > > > addr_size check with size of sa_family_t from __sctp_connect()
> > > > > > for the 1st address.
> > > > > >
> > > > > > So before doing that, __sctp_setsockopt_connectx() should do
> > > > > > this check first, as sctp_inet_connect() does.
> > > > > >
> > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > ---
> > > > > > net/sctp/socket.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > > > index aa80cda..5f92e4a 100644
> > > > > > --- a/net/sctp/socket.c
> > > > > > +++ b/net/sctp/socket.c
> > > > > > @@ -1311,7 +1311,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
> > > > > > pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
> > > > > > __func__, sk, addrs, addrs_size);
> > > > > >
> > > > > > - if (unlikely(addrs_size <= 0))
> > > > > > + if (unlikely(addrs_size < sizeof(sa_family_t)))
> > > > > I don't think this is what you want to check for here. sa_family_t is
> > > > > an unsigned short, and addrs_size is the number of bytes in the addrs
> > > > > array. The addrs array should be at least the size of one struct
> > > > > sockaddr (16 bytes iirc), and, if larger, should be a multiple of
> > > > > sizeof(struct sockaddr)
> > > > sizeof(struct sockaddr) is not the right value to check either.
> > > >
> > > > The proper check will be done later in __sctp_connect():
> > > >
> > > > af = sctp_get_af_specific(daddr->sa.sa_family);
> > > > if (!af || af->sockaddr_len > addrs_size)
> > > > return -EINVAL;
> > > >
> > > > So the check 'addrs_size < sizeof(sa_family_t)' in this patch is
> > > > just to make sure daddr->sa.sa_family is accessible. the same
> > > > check is also done in sctp_inet_connect().
> > > >
> > > That doesn't make much sense, if the proper check is done in __sctp_connect with
> > > the size of the families sockaddr_len, then we don't need this check at all, we
> > > can just let memdup_user take the fault on copy_to_user and return -EFAULT. If
> > > we get that from memdup_user, we know its not accessible, and can bail out.
> > >
> > > About the only thing we need to check for here is that addr_len isn't some
> > > absurdly high value (i.e. a negative value), so that we avoid trying to kmalloc
> > > upwards of 2G in memdup_user. Your change does that just fine, but its no
> > > better or worse than checking for <=0
> >
> > One can argue that such check against absurdly high values is random
> > and not effective, as 2G can be somewhat reasonable on 8GB systems but
> > certainly isn't on 512MB ones. On that, kmemdup_user() will also fail
> > gracefully as it uses GFP_USER and __GFP_NOWARN.
> >
> > The original check is more for protecting for sane usage of the
> > variable, which is an int, and a negative value is questionable. We
> > could cast, yes, but.. was that really the intent of the application?
> > Probably not.
>
> Though that said, I'm okay with the new check here: a quick sanity
> check that can avoid expensive calls to kmalloc(), while more refined
> check is done later on.
>
I agree a sanity check makes sense, just to avoid allocating a huge value
(even 2G is absurd on many systems), however, I'm not super comfortable with
checking for the value being less than 16 (sizeof(sa_family_t)). The zero check
is fairly obvious given the signed nature of the lengh field, this check makes
me wonder what exactly we are checking for.
Neil
> >
> > >
> > > Neil
> > >
> > > > >
> > > > > Neil
> > > > >
> > > > > > return -EINVAL;
> > > > > >
> > > > > > kaddrs = memdup_user(addrs, addrs_size);
> > > > > > --
> > > > > > 2.1.0
> > > > > >
> > > > > >
> > > >
>
^ permalink raw reply
* Re: [PATCH net-next 00/10] Use dev_get_drvdata where possible
From: David Miller @ 2019-07-24 18:45 UTC (permalink / raw)
To: hslester96
Cc: mlindner, stephen, jirislaby, mickflemm, mcgrof, sgruszka, kvalo,
ath9k-devel, merez, johannes.berg, emmanuel.grumbach,
luciano.coelho, amitkarwar, nishants, gbhat, huxinming820,
imitsyanko, avinashp, smatyukevich, pkshih, linuxwifi,
linux-net-drivers, ecree, mhabets, netdev, wil6210,
linux-wireless, linux-kernel
In-Reply-To: <20190724112524.13042-1-hslester96@gmail.com>
From: Chuhong Yuan <hslester96@gmail.com>
Date: Wed, 24 Jul 2019 19:25:24 +0800
> These patches use dev_get_drvdata instead of
> using to_pci_dev + pci_get_drvdata to make
> code simpler.
Patches 1-4 applied to net-nex t.
^ permalink raw reply
* Re: [PATCH] Build fixes for skb_frag_size conversion
From: David Miller @ 2019-07-24 18:46 UTC (permalink / raw)
To: willy; +Cc: netdev
In-Reply-To: <20190724113615.11961-1-willy@infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Wed, 24 Jul 2019 04:36:15 -0700
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> I missed a few places. One is in some ifdeffed code which will probably
> never be re-enabled; the others are in drivers which can't currently be
> compiled on x86.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Applied, thanks for following up.
^ permalink raw reply
* Re: [PATCH v3 6/7] net: Rename skb_frag_t size to bv_len
From: David Miller @ 2019-07-24 18:46 UTC (permalink / raw)
To: willy; +Cc: David.Laight, hch, netdev
In-Reply-To: <20190724114120.GT363@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Wed, 24 Jul 2019 04:41:21 -0700
> On Wed, Jul 24, 2019 at 10:49:03AM +0000, David Laight wrote:
>> This is 'just plain stupid'.
>> The 'bv_' prefix of the members of 'struct bvec' is there so that 'grep'
>> (etc) can be used to find the uses of the members.
>>
>> In a 'struct skb_frag_struct' a sensible prefix might be 'sf_'.
>>
>> OTOH it might be sensible to use (or embed) a 'struct bvec'
>> instead of 'skb_frag_struct'.
>
> Maybe you should read patch 7/7. Or 0/7.
+1
^ permalink raw reply
* Re: [PATCH net-next 0/4] sctp: clean up __sctp_connect function
From: Neil Horman @ 2019-07-24 18:47 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: Xin Long, network dev, linux-sctp, davem
In-Reply-To: <20190724142512.GG6204@localhost.localdomain>
On Wed, Jul 24, 2019 at 11:25:12AM -0300, Marcelo Ricardo Leitner wrote:
> On Tue, Jul 23, 2019 at 01:37:56AM +0800, Xin Long wrote:
> > This patchset is to factor out some common code for
> > sctp_sendmsg_new_asoc() and __sctp_connect() into 2
> > new functioins.
> >
> > Xin Long (4):
> > sctp: check addr_size with sa_family_t size in
> > __sctp_setsockopt_connectx
> > sctp: clean up __sctp_connect
> > sctp: factor out sctp_connect_new_asoc
> > sctp: factor out sctp_connect_add_peer
>
> Nice cleanup! These patches LGTM. Hopefully for Neil as well.
>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
Yes, agreed, this all looks good, but I would like to resolve the addr_length
check issue before I ack it.
Neil
^ permalink raw reply
* Re: [PATCH net-next v3 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: David Miller @ 2019-07-24 18:52 UTC (permalink / raw)
To: antoine.tenart
Cc: richardcochran, alexandre.belloni, UNGLinuxDriver, ralf,
paul.burton, jhogan, netdev, linux-mips, thomas.petazzoni,
allan.nielsen
In-Reply-To: <20190724081715.29159-9-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Wed, 24 Jul 2019 10:17:15 +0200
> +static int ocelot_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> + u32 unit = 0, direction = 0;
> + unsigned long flags;
^^^^
> + u64 adj = 0;
> +
> + if (!scaled_ppm)
> + goto disable_adj;
...
> +disable_adj:
> + ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> +
> + spin_unlock_irqrestore(&ocelot->ptp_clock_lock, flags);
^^^^^
Did GCC really not warn about this in your build like it did immediately
on mine?
drivers/net/ethernet/mscc/ocelot.c: In function ‘ocelot_ptp_adjfine’:
./include/linux/spinlock.h:288:3: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized]
_raw_spin_unlock_irqrestore(lock, flags); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Please fix this and when you respin please just elide the MIPS tree
patches and just keep all the ones that I should apply to net-next.
Thank you.
^ permalink raw reply
* Re: Reminder: 99 open syzbot bugs in net subsystem
From: Eric Dumazet @ 2019-07-24 18:52 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Dmitry Vyukov, netdev,
Florian Westphal, i.maximets, Eric Dumazet, David Ahern, LKML,
syzkaller-bugs
In-Reply-To: <20190724183710.GF213255@gmail.com>
On Wed, Jul 24, 2019 at 8:37 PM Eric Biggers <ebiggers@kernel.org> wrote:
> A huge number of valid open bugs are not being fixed, which is a fact. We can
> argue about what words to use to describe this situation, but it doesn't change
> the situation itself.
>
> What is your proposed solution?
syzbot sends emails, plenty of them, with many wrong bisection
results, increasing the noise.
If nobody is interested, I am not sure sending copies of them
repeatedly will be of any help.
Maybe a simple monthly reminder with one URL to go to the list of bugs
would be less intrusive.
^ permalink raw reply
* Re: Reminder: 99 open syzbot bugs in net subsystem
From: Eric Biggers @ 2019-07-24 19:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Eric Dumazet, Dmitry Vyukov, netdev,
Florian Westphal, i.maximets, David Ahern, LKML, syzkaller-bugs
In-Reply-To: <CANn89iKZcdk-YfqZ-F1toHDLW3Etf5oPR78bXOq0FbjwWyiSMQ@mail.gmail.com>
On Wed, Jul 24, 2019 at 08:52:54PM +0200, 'Eric Dumazet' via syzkaller-bugs wrote:
> On Wed, Jul 24, 2019 at 8:37 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> > A huge number of valid open bugs are not being fixed, which is a fact. We can
> > argue about what words to use to describe this situation, but it doesn't change
> > the situation itself.
> >
> > What is your proposed solution?
>
> syzbot sends emails, plenty of them, with many wrong bisection
> results, increasing the noise.
>
> If nobody is interested, I am not sure sending copies of them
> repeatedly will be of any help.
>
> Maybe a simple monthly reminder with one URL to go to the list of bugs
> would be less intrusive.
>
The bogus bisection results is a known issue (which I'm trying to convince
Dmitry is important enough to fix...), which is why I manually reviewed all of
them and discarded out all the obviously incorrect ones. My reminders only
include manually reviewed bisection results. Obviously there will still be some
looked plausible but are actualy wrong, but I suspect the accuracy is around
80-90% rather than the 40-50% of the raw syzbot bisection results.
- Eric
^ permalink raw reply
* Re: [PATCH net-next 1/4] sctp: check addr_size with sa_family_t size in __sctp_setsockopt_connectx
From: Marcelo Ricardo Leitner @ 2019-07-24 19:05 UTC (permalink / raw)
To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem
In-Reply-To: <20190724184456.GC7212@hmswarspite.think-freely.org>
On Wed, Jul 24, 2019 at 02:44:56PM -0400, Neil Horman wrote:
> On Wed, Jul 24, 2019 at 09:49:07AM -0300, Marcelo Ricardo Leitner wrote:
> > On Wed, Jul 24, 2019 at 09:36:50AM -0300, Marcelo Ricardo Leitner wrote:
> > > On Wed, Jul 24, 2019 at 07:22:35AM -0400, Neil Horman wrote:
> > > > On Wed, Jul 24, 2019 at 03:21:12PM +0800, Xin Long wrote:
> > > > > On Tue, Jul 23, 2019 at 11:25 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 23, 2019 at 01:37:57AM +0800, Xin Long wrote:
> > > > > > > Now __sctp_connect() is called by __sctp_setsockopt_connectx() and
> > > > > > > sctp_inet_connect(), the latter has done addr_size check with size
> > > > > > > of sa_family_t.
> > > > > > >
> > > > > > > In the next patch to clean up __sctp_connect(), we will remove
> > > > > > > addr_size check with size of sa_family_t from __sctp_connect()
> > > > > > > for the 1st address.
> > > > > > >
> > > > > > > So before doing that, __sctp_setsockopt_connectx() should do
> > > > > > > this check first, as sctp_inet_connect() does.
> > > > > > >
> > > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > > ---
> > > > > > > net/sctp/socket.c | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > > > > index aa80cda..5f92e4a 100644
> > > > > > > --- a/net/sctp/socket.c
> > > > > > > +++ b/net/sctp/socket.c
> > > > > > > @@ -1311,7 +1311,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
> > > > > > > pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
> > > > > > > __func__, sk, addrs, addrs_size);
> > > > > > >
> > > > > > > - if (unlikely(addrs_size <= 0))
> > > > > > > + if (unlikely(addrs_size < sizeof(sa_family_t)))
> > > > > > I don't think this is what you want to check for here. sa_family_t is
> > > > > > an unsigned short, and addrs_size is the number of bytes in the addrs
> > > > > > array. The addrs array should be at least the size of one struct
> > > > > > sockaddr (16 bytes iirc), and, if larger, should be a multiple of
> > > > > > sizeof(struct sockaddr)
> > > > > sizeof(struct sockaddr) is not the right value to check either.
> > > > >
> > > > > The proper check will be done later in __sctp_connect():
> > > > >
> > > > > af = sctp_get_af_specific(daddr->sa.sa_family);
> > > > > if (!af || af->sockaddr_len > addrs_size)
> > > > > return -EINVAL;
> > > > >
> > > > > So the check 'addrs_size < sizeof(sa_family_t)' in this patch is
> > > > > just to make sure daddr->sa.sa_family is accessible. the same
> > > > > check is also done in sctp_inet_connect().
> > > > >
> > > > That doesn't make much sense, if the proper check is done in __sctp_connect with
> > > > the size of the families sockaddr_len, then we don't need this check at all, we
> > > > can just let memdup_user take the fault on copy_to_user and return -EFAULT. If
> > > > we get that from memdup_user, we know its not accessible, and can bail out.
> > > >
> > > > About the only thing we need to check for here is that addr_len isn't some
> > > > absurdly high value (i.e. a negative value), so that we avoid trying to kmalloc
> > > > upwards of 2G in memdup_user. Your change does that just fine, but its no
> > > > better or worse than checking for <=0
> > >
> > > One can argue that such check against absurdly high values is random
> > > and not effective, as 2G can be somewhat reasonable on 8GB systems but
> > > certainly isn't on 512MB ones. On that, kmemdup_user() will also fail
> > > gracefully as it uses GFP_USER and __GFP_NOWARN.
> > >
> > > The original check is more for protecting for sane usage of the
> > > variable, which is an int, and a negative value is questionable. We
> > > could cast, yes, but.. was that really the intent of the application?
> > > Probably not.
> >
> > Though that said, I'm okay with the new check here: a quick sanity
> > check that can avoid expensive calls to kmalloc(), while more refined
> > check is done later on.
> >
> I agree a sanity check makes sense, just to avoid allocating a huge value
> (even 2G is absurd on many systems), however, I'm not super comfortable with
> checking for the value being less than 16 (sizeof(sa_family_t)). The zero check
16 bits you mean then, per
include/uapi/linux/socket.h
typedef unsigned short __kernel_sa_family_t;
include/linux/socket.h
typedef __kernel_sa_family_t sa_family_t;
> is fairly obvious given the signed nature of the lengh field, this check makes
> me wonder what exactly we are checking for.
A minimum viable buffer without doing more extensive tests. Beyond
sa_family, we need to parse sa_family and then that's left for later.
Perhaps a comment helps, something like
/* Check if we have at least the family type in there */
?
Marcelo
>
> Neil
>
> > >
> > > >
> > > > Neil
> > > >
> > > > > >
> > > > > > Neil
> > > > > >
> > > > > > > return -EINVAL;
> > > > > > >
> > > > > > > kaddrs = memdup_user(addrs, addrs_size);
> > > > > > > --
> > > > > > > 2.1.0
> > > > > > >
> > > > > > >
> > > > >
> >
^ permalink raw reply
* Re: [PATCH net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: Saeed Mahameed @ 2019-07-24 19:12 UTC (permalink / raw)
To: tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
liuyonglong@huawei.com
In-Reply-To: <1563938327-9865-8-git-send-email-tanhuazhong@huawei.com>
On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
>
> Some times just see the eth interface have been down/up via
> dmesg, but can not know why the eth down. So adds some debug
> messages to identify the cause for this.
>
I really don't like this. your default msg lvl has NETIF_MSG_IFDOWN
turned on .. dumping every single operation that happens on your device
by default to kernel log is too much !
We should really consider using trace buffers with well defined
structures for vendor specific events. so we can use bpf filters and
state of the art tools for netdev debugging.
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 24
> ++++++++++++++++++++
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 26
> ++++++++++++++++++++++
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 14 ++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 4d58c53..cff5d59 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -459,6 +459,10 @@ static int hns3_nic_net_open(struct net_device
> *netdev)
> h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
>
> hns3_config_xps(priv);
> +
> + if (netif_msg_ifup(h))
> + netdev_info(netdev, "net open\n");
> +
> return 0;
> }
>
> @@ -519,6 +523,9 @@ static int hns3_nic_net_stop(struct net_device
> *netdev)
> if (test_and_set_bit(HNS3_NIC_STATE_DOWN, &priv->state))
> return 0;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev, "net stop\n");
> +
> if (h->ae_algo->ops->set_timer_task)
> h->ae_algo->ops->set_timer_task(priv->ae_handle,
> false);
>
> @@ -1550,6 +1557,9 @@ static int hns3_setup_tc(struct net_device
> *netdev, void *type_data)
> h = hns3_get_handle(netdev);
> kinfo = &h->kinfo;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev, "setup tc: num_tc=%d\n", tc);
> +
> return (kinfo->dcb_ops && kinfo->dcb_ops->setup_tc) ?
> kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : -EOPNOTSUPP;
> }
> @@ -1593,6 +1603,11 @@ static int hns3_ndo_set_vf_vlan(struct
> net_device *netdev, int vf, u16 vlan,
> struct hnae3_handle *h = hns3_get_handle(netdev);
> int ret = -EIO;
>
> + if (netif_msg_ifdown(h))
why msg_ifdown ? looks like netif_msg_drv is more appropriate, for many
of the cases in this patch.
> + netdev_info(netdev,
> + "set vf vlan: vf=%d, vlan=%d, qos=%d,
> vlan_proto=%d\n",
> + vf, vlan, qos, vlan_proto);
> +
> if (h->ae_algo->ops->set_vf_vlan_filter)
> ret = h->ae_algo->ops->set_vf_vlan_filter(h, vf, vlan,
> qos,
> vlan_proto);
> @@ -1611,6 +1626,10 @@ static int hns3_nic_change_mtu(struct
> net_device *netdev, int new_mtu)
> if (!h->ae_algo->ops->set_mtu)
> return -EOPNOTSUPP;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev, "change mtu from %d to %d\n",
> + netdev->mtu, new_mtu);
> +
> ret = h->ae_algo->ops->set_mtu(h, new_mtu);
> if (ret)
> netdev_err(netdev, "failed to change MTU in hardware
> %d\n",
> @@ -4395,6 +4414,11 @@ int hns3_set_channels(struct net_device
> *netdev,
> if (kinfo->rss_size == new_tqp_num)
> return 0;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev,
> + "set channels: tqp_num=%d, rxfh=%d\n",
> + new_tqp_num, rxfh_configured);
> +
> ret = hns3_reset_notify(h, HNAE3_DOWN_CLIENT);
> if (ret)
> return ret;
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> index e71c92b..edb9845 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
> @@ -311,6 +311,9 @@ static void hns3_self_test(struct net_device
> *ndev,
> if (eth_test->flags != ETH_TEST_FL_OFFLINE)
> return;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(ndev, "self test start\n");
> +
> st_param[HNAE3_LOOP_APP][0] = HNAE3_LOOP_APP;
> st_param[HNAE3_LOOP_APP][1] =
> h->flags & HNAE3_SUPPORT_APP_LOOPBACK;
> @@ -374,6 +377,9 @@ static void hns3_self_test(struct net_device
> *ndev,
>
> if (if_running)
> ndev->netdev_ops->ndo_open(ndev);
> +
> + if (netif_msg_ifdown(h))
> + netdev_info(ndev, "self test end\n");
> }
>
> static int hns3_get_sset_count(struct net_device *netdev, int
> stringset)
> @@ -604,6 +610,11 @@ static int hns3_set_pauseparam(struct net_device
> *netdev,
> {
> struct hnae3_handle *h = hns3_get_handle(netdev);
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev,
> + "set pauseparam: autoneg=%d, rx:%d,
> tx:%d\n",
> + param->autoneg, param->rx_pause, param-
> >tx_pause);
> +
> if (h->ae_algo->ops->set_pauseparam)
> return h->ae_algo->ops->set_pauseparam(h, param-
> >autoneg,
> param->rx_pause,
> @@ -743,6 +754,13 @@ static int hns3_set_link_ksettings(struct
> net_device *netdev,
> if (cmd->base.speed == SPEED_1000 && cmd->base.duplex ==
> DUPLEX_HALF)
> return -EINVAL;
>
> + if (netif_msg_ifdown(handle))
> + netdev_info(netdev,
> + "set link(%s): autoneg=%d, speed=%d,
> duplex=%d\n",
> + netdev->phydev ? "phy" : "mac",
> + cmd->base.autoneg, cmd->base.speed,
> + cmd->base.duplex);
> +
> /* Only support ksettings_set for netdev with phy attached for
> now */
> if (netdev->phydev)
> return phy_ethtool_ksettings_set(netdev->phydev, cmd);
> @@ -984,6 +1002,10 @@ static int hns3_nway_reset(struct net_device
> *netdev)
> return -EINVAL;
> }
>
> + if (netif_msg_ifdown(handle))
> + netdev_info(netdev, "nway reset (using %s)\n",
> + phy ? "phy" : "mac");
> +
> if (phy)
> return genphy_restart_aneg(phy);
>
> @@ -1308,6 +1330,10 @@ static int hns3_set_fecparam(struct net_device
> *netdev,
> if (!ops->set_fec)
> return -EOPNOTSUPP;
> fec_mode = eth_to_loc_fec(fec->fec);
> +
> + if (netif_msg_ifdown(handle))
> + netdev_info(netdev, "set fecparam: mode=%d\n",
> fec_mode);
> +
> return ops->set_fec(handle, fec_mode);
> }
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
> index bac4ce1..133e7c6 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
> @@ -201,6 +201,7 @@ static int hclge_client_setup_tc(struct hclge_dev
> *hdev)
> static int hclge_ieee_setets(struct hnae3_handle *h, struct ieee_ets
> *ets)
> {
> struct hclge_vport *vport = hclge_get_vport(h);
> + struct net_device *netdev = h->kinfo.netdev;
> struct hclge_dev *hdev = vport->back;
> bool map_changed = false;
> u8 num_tc = 0;
> @@ -215,6 +216,9 @@ static int hclge_ieee_setets(struct hnae3_handle
> *h, struct ieee_ets *ets)
> return ret;
>
> if (map_changed) {
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev, "set ets\n");
> +
> ret = hclge_notify_client(hdev, HNAE3_DOWN_CLIENT);
> if (ret)
> return ret;
> @@ -300,6 +304,7 @@ static int hclge_ieee_getpfc(struct hnae3_handle
> *h, struct ieee_pfc *pfc)
> static int hclge_ieee_setpfc(struct hnae3_handle *h, struct ieee_pfc
> *pfc)
> {
> struct hclge_vport *vport = hclge_get_vport(h);
> + struct net_device *netdev = h->kinfo.netdev;
> struct hclge_dev *hdev = vport->back;
> u8 i, j, pfc_map, *prio_tc;
>
> @@ -325,6 +330,11 @@ static int hclge_ieee_setpfc(struct hnae3_handle
> *h, struct ieee_pfc *pfc)
> hdev->tm_info.hw_pfc_map = pfc_map;
> hdev->tm_info.pfc_en = pfc->pfc_en;
>
> + if (netif_msg_ifdown(h))
> + netdev_info(netdev,
> + "set pfc: pfc_en=%d, pfc_map=%d,
> num_tc=%d\n",
> + pfc->pfc_en, pfc_map, hdev-
> >tm_info.num_tc);
> +
> hclge_tm_pfc_info_update(hdev);
>
> return hclge_pause_setup_hw(hdev, false);
> @@ -345,8 +355,12 @@ static u8 hclge_getdcbx(struct hnae3_handle *h)
> static u8 hclge_setdcbx(struct hnae3_handle *h, u8 mode)
> {
> struct hclge_vport *vport = hclge_get_vport(h);
> + struct net_device *netdev = h->kinfo.netdev;
> struct hclge_dev *hdev = vport->back;
>
> + if (netif_msg_drv(h))
> + netdev_info(netdev, "set dcbx: mode=%d\n", mode);
> +
> /* No support for LLD_MANAGED modes or CEE */
> if ((mode & DCB_CAP_DCBX_LLD_MANAGED) ||
> (mode & DCB_CAP_DCBX_VER_CEE) ||
^ 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