* Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port
From: Vasundhara Volam @ 2019-01-24 9:46 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Michal Kubecek, Netdev, David Miller, michael.chan@broadcom.com,
Jiri Pirko
In-Reply-To: <20190122141842.26908b27@cakuba.netronome.com>
On Wed, Jan 23, 2019 at 3:48 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 18 Jan 2019 15:33:19 +0100, Michal Kubecek wrote:
> > On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > > There is difference of opinion on adding WOL parameter to devlink, between
> > > Jakub Kicinski and Michael Chan.
> > >
> > > Quote from Jakud Kicinski:
> > > ********
> > > As explained previously I think it's a very bad idea to add existing
> > > configuration options to devlink, just because devlink has the ability
> > > to persist the setting in NVM. Especially that for WoL you have to get
> > > the link up so you potentially have all link config stuff as well. And
> > > that n-tuple filters are one of the WoL options, meaning we'd need the
> > > ability to persist n-tuple filters via devlink.
> > >
> > > The effort would be far better spent helping with migrating ethtool to
> > > netlink, and allowing persisting there.
> > >
> > > I have not heard any reason why devlink is a better fit. I can imagine
> > > you're just doing it here because it's less effort for you since
> > > ethtool is not yet migrated.
> > > ********
> > >
> > > Quote from Michael Chan:
> > > ********
> > > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > > including n-tuple with IP address in addition to the more basic types
> > > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > > magic packet and other simple types.
> >
> > If I understand correctly, it's that way now. I'm not sure there is a
> > technical reason preventing more complex WoL types in the OS-absent case
> > in the future. Also, even with traditional ethtool WoL setting, most
> > NICs only support some of the types (I'm not sure if there is a NIC
> > which would support all of them.)
> >
> > > The devlink WoL setting does not have to match the ethtool WoL
> > > setting.
> >
> > IMHO this is not really a problem. We can either use an additional flag
> > telling kernel/driver if we are setting runtime or persistent WoL mask
> > or we can pass (up to) two bitmaps.
>
> I think reusing new netlink ethtool with a special flag would be a nice,
> complete solution. We could also address link settings this way (which
> are a pre-requisite for WoL).
>
> I have no strong preference on the mechanism, but for ease of setting
> as well as dumping would it be workable to use a nesting, like this:
>
> Run time settings:
> [ETHTOOL_SETTINGS_BLA]
> [ETHTOOL_BLA_VAL_1]
> [ETHTOOL_BLA_VAL_2]
> ...
>
> Persistent:
> [ETHTOOL_PERSISTENT]
> [ETHTOOL_SETTINGS_BLA]
> [ETHTOOL_BLA_VAL_1]
> [ETHTOOL_BLA_VAL_2]
>
> IOW encapsulate settings into a "persistent" attribute?
Not sure if current devlink framework allows to encapsulate additional
settings now.
But we can think of extending it to support this when there is a requirement.
>
> How does that look to you, Michal?
>
> > > The card will autoneg up to the speed supported by Vaux so no special
> > > devlink link setting is needed.
> > > ********
> >
> > Like Jakub, I'm not convinced there is a strong technical reason to have
> > each of the WoL settings handled through a different interface. I don't
> > say, though, that ethtool is necessarily the right one. If there is
> > a consensus that it better fits into devlink, I can imagine that both
> > could be accessible through devlink (for start, in drivers which choose
> > so, e.g. because they want to implement the persistent setting).
> >
> > Michal Kubecek
^ permalink raw reply
* Re: [PATCH v5 4/4] Bluetooth: btusb: Use the cmd_timeout method to reset the Intel BT chip
From: Marcel Holtmann @ 2019-01-24 9:46 UTC (permalink / raw)
To: Rajat Jain
Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
Dmitry Torokhov, Alex Hung, linux-bluetooth, linux-kernel,
linux-usb, netdev, rajatxjain, dtor, raghuram.hegde,
chethan.tumkur.narayan, sukumar.ghorai
In-Reply-To: <20190123205725.239661-4-rajatja@google.com>
Hi Rajat,
> If the platform provides it, use the reset gpio to reset the Intel BT
> chip, as part of cmd_timeout handling. This has been found helpful on
> Intel bluetooth controllers where the firmware gets stuck and the only
> way out is a hard reset pin provided by the platform.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v5: Rename the hook to cmd_timeout, and wait for 5 timeouts before
> resetting the device.
> v4: Use data->flags instead of clearing the quirk in btusb_hw_reset()
> v3: Better error handling for gpiod_get_optional()
> v2: Handle the EPROBE_DEFER case.
>
> drivers/bluetooth/btusb.c | 54 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 4761499db9ee..8949ea30a1bc 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -29,6 +29,7 @@
> #include <linux/of_device.h>
> #include <linux/of_irq.h>
> #include <linux/suspend.h>
> +#include <linux/gpio/consumer.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -439,6 +440,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = {
> #define BTUSB_BOOTING 9
> #define BTUSB_DIAG_RUNNING 10
> #define BTUSB_OOB_WAKE_ENABLED 11
> +#define BTUSB_HW_RESET_ACTIVE 12
>
> struct btusb_data {
> struct hci_dev *hdev;
> @@ -476,6 +478,8 @@ struct btusb_data {
> struct usb_endpoint_descriptor *diag_tx_ep;
> struct usb_endpoint_descriptor *diag_rx_ep;
>
> + struct gpio_desc *reset_gpio;
> +
> __u8 cmdreq_type;
> __u8 cmdreq;
>
> @@ -489,8 +493,39 @@ struct btusb_data {
> int (*setup_on_usb)(struct hci_dev *hdev);
>
> int oob_wake_irq; /* irq for out-of-band wake-on-bt */
> + unsigned num_cmd_timeout;
Make this cmd_timeout_count or cmd_timeout_cnt.
> };
>
> +
> +static void btusb_intel_cmd_timeout(struct hci_dev *hdev,
> + struct hci_command_hdr *cmd)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + struct gpio_desc *reset_gpio = data->reset_gpio;
> +
> + bt_dev_err(hdev, "btusb num_cmd_timeout = %u", ++data->num_cmd_timeout);
I would prefer if you don’t do the increase in the bt_dev_err(). And you can also scrap the “btusb “ prefix here since that is all present via bt_dev_err if really needed at some point. Actually I question the whole bt_dev_err here in the first place since the core will put our the error. You are just repeating it here.
> +
> + if (data->num_cmd_timeout < 5)
> + return;
So I would propose to do just this:
if (++data->cmd_timeout_count < 5)
return;
> +
> + /*
> + * Toggle the hard reset line if the platform provides one. The reset
> + * is going to yank the device off the USB and then replug. So doing
> + * once is enough. The cleanup is handled correctly on the way out
> + * (standard USB disconnect), and the new device is detected cleanly
> + * and bound to the driver again like it should be.
> + */
> + if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> + bt_dev_err(hdev, "last reset failed? Not resetting again");
> + return;
> + }
> +
> + bt_dev_err(hdev, "Initiating HW reset via gpio");
> + gpiod_set_value(reset_gpio, 1);
> + mdelay(100);
> + gpiod_set_value(reset_gpio, 0);
> +}
> +
> static inline void btusb_free_frags(struct btusb_data *data)
> {
> unsigned long flags;
> @@ -2915,6 +2950,7 @@ static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> struct usb_endpoint_descriptor *ep_desc;
> + struct gpio_desc *reset_gpio;
> struct btusb_data *data;
> struct hci_dev *hdev;
> unsigned ifnum_base;
> @@ -3028,6 +3064,15 @@ static int btusb_probe(struct usb_interface *intf,
>
> SET_HCIDEV_DEV(hdev, &intf->dev);
>
> + reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> + GPIOD_OUT_LOW);
Any reason to not use the devm_gpiod_get_optional() version here?
> + if (IS_ERR(reset_gpio)) {
> + err = PTR_ERR(reset_gpio);
> + goto out_free_dev;
> + } else if (reset_gpio) {
> + data->reset_gpio = reset_gpio;
> + }
> +
> hdev->open = btusb_open;
> hdev->close = btusb_close;
> hdev->flush = btusb_flush;
> @@ -3085,6 +3130,8 @@ static int btusb_probe(struct usb_interface *intf,
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + if (data->reset_gpio)
> + hdev->cmd_timeout = btusb_intel_cmd_timeout;
> }
Always assign hdev->cmd_timeout = btusb_intel_cmd_timeout and put it after btintel_set_bdaddr and before the quirks.
Move the if (data->reset_gpio) check into btusb_intel_cmd_timeout() function and print a warning that no reset GPIO is present.
>
> if (id->driver_info & BTUSB_INTEL_NEW) {
> @@ -3097,6 +3144,8 @@ static int btusb_probe(struct usb_interface *intf,
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + if (data->reset_gpio)
> + hdev->cmd_timeout = btusb_intel_cmd_timeout;
> }
>
> if (id->driver_info & BTUSB_MARVELL)
> @@ -3226,6 +3275,8 @@ static int btusb_probe(struct usb_interface *intf,
> return 0;
>
> out_free_dev:
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> hci_free_dev(hdev);
> return err;
> }
> @@ -3269,6 +3320,9 @@ static void btusb_disconnect(struct usb_interface *intf)
> if (data->oob_wake_irq)
> device_init_wakeup(&data->udev->dev, false);
>
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> +
> hci_free_dev(hdev);
> }
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port
From: Vasundhara Volam @ 2019-01-24 9:42 UTC (permalink / raw)
To: Michal Kubecek
Cc: Netdev, David Miller, michael.chan@broadcom.com, Jiri Pirko,
Jakub Kicinski
In-Reply-To: <20190118143319.GG26670@unicorn.suse.cz>
On Fri, Jan 18, 2019 at 8:03 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > There is difference of opinion on adding WOL parameter to devlink, between
> > Jakub Kicinski and Michael Chan.
> >
> > Quote from Jakud Kicinski:
> > ********
> > As explained previously I think it's a very bad idea to add existing
> > configuration options to devlink, just because devlink has the ability
> > to persist the setting in NVM. Especially that for WoL you have to get
> > the link up so you potentially have all link config stuff as well. And
> > that n-tuple filters are one of the WoL options, meaning we'd need the
> > ability to persist n-tuple filters via devlink.
> >
> > The effort would be far better spent helping with migrating ethtool to
> > netlink, and allowing persisting there.
> >
> > I have not heard any reason why devlink is a better fit. I can imagine
> > you're just doing it here because it's less effort for you since
> > ethtool is not yet migrated.
> > ********
> >
> > Quote from Michael Chan:
> > ********
> > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > including n-tuple with IP address in addition to the more basic types
> > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > magic packet and other simple types.
>
> If I understand correctly, it's that way now. I'm not sure there is a
> technical reason preventing more complex WoL types in the OS-absent case
> in the future. Also, even with traditional ethtool WoL setting, most
> NICs only support some of the types (I'm not sure if there is a NIC
> which would support all of them.)
Agreed. This can be extended in future for other types of WoL.
>
> > The devlink WoL setting does not have to match the ethtool WoL
> > setting.
>
> IMHO this is not really a problem. We can either use an additional flag
> telling kernel/driver if we are setting runtime or persistent WoL mask
> or we can pass (up to) two bitmaps.
This already supports as persistent or runtime WoL. All devlink parameters
have this option to be either persistent or runtime which is defined
by the driver
when defining the parameter.
>
> > The card will autoneg up to the speed supported by Vaux so no special
> > devlink link setting is needed.
> > ********
>
> Like Jakub, I'm not convinced there is a strong technical reason to have
> each of the WoL settings handled through a different interface. I don't
> say, though, that ethtool is necessarily the right one. If there is
> a consensus that it better fits into devlink, I can imagine that both
> could be accessible through devlink (for start, in drivers which choose
> so, e.g. because they want to implement the persistent setting).
Devlink supports both persistent and runtime WoL setting. It depends on
driver's use case.
>
> Michal Kubecek
^ permalink raw reply
* Re: [PATCH v5 3/4] Bluetooth: Allow driver specific cmd timeout handling
From: Marcel Holtmann @ 2019-01-24 9:36 UTC (permalink / raw)
To: Rajat Jain
Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
Dmitry Torokhov, Alex Hung, Bluez mailing list,
Linux Kernel Mailing List, linux-usb, netdev, rajatxjain, dtor,
Raghuram Hegde, chethan.tumkur.narayan, sukumar.ghorai
In-Reply-To: <20190123205725.239661-3-rajatja@google.com>
Hi Rajat,
> Add a hook to allow the BT driver to do device or command specific
> handling in case of timeouts. This is to be used by Intel driver to
> reset the device after certain number of timeouts.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v5: Drop the quirk, and rename the hook function to cmd_timeout()
> v4: same as v1
> v3: same as v1
> v2: same as v1
>
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 10 ++++++++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e5ea633ea368..624d5f2b1f36 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -437,6 +437,7 @@ struct hci_dev {
> int (*post_init)(struct hci_dev *hdev);
> int (*set_diag)(struct hci_dev *hdev, bool enable);
> int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> + void (*cmd_timeout)(struct hci_dev *hdev, struct hci_command_hdr *cmd);
> };
>
> #define HCI_PHY_HANDLE(handle) (handle & 0xff)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7352fe85674b..c6917f57581a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2568,16 +2568,22 @@ static void hci_cmd_timeout(struct work_struct *work)
> {
> struct hci_dev *hdev = container_of(work, struct hci_dev,
> cmd_timer.work);
> + struct hci_command_hdr *sent = NULL;
>
> if (hdev->sent_cmd) {
> - struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> - u16 opcode = __le16_to_cpu(sent->opcode);
> + u16 opcode;
> +
> + sent = (void *) hdev->sent_cmd->data;
> + opcode = __le16_to_cpu(sent->opcode);
>
> bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
> } else {
> bt_dev_err(hdev, "command tx timeout");
> }
>
> + if (hdev->cmd_timeout)
> + hdev->cmd_timeout(hdev, sent);
> +
drop the sent parameter. You are not using it and if at some point in the future you might want to use it, then we add it and fix up all users.
And frankly, I would move the hdev->cmd_timeout call into the hdev->sent_cmd block since I do not even know if it makes sense to deal with the fallback case where hdev->sent_cmd is not available. Unless you have that kind of error case, but that is only possible if you have external injection of HCI commands.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next 06/10] net: introduce a net_device_ops macsec helper
From: Antoine Tenart @ 2019-01-24 9:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, sd, andrew, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <6ebf0541-0830-3df9-121f-ac560822bf1c@gmail.com>
Hi Florian,
On Wed, Jan 23, 2019 at 12:16:08PM -0800, Florian Fainelli wrote:
> On 1/23/19 7:56 AM, Antoine Tenart wrote:
> > This patch introduces a net_device_ops MACsec helper to allow net device
> > drivers to implement a MACsec offloading solution.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> > ---
> > include/linux/netdevice.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index e675ef97a426..ee2f40dca515 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -53,6 +53,10 @@
> > #include <uapi/linux/pkt_cls.h>
> > #include <linux/hashtable.h>
> >
> > +#ifdef CONFIG_MACSEC
> > +#include <net/macsec.h>
> > +#endif
>
> You can provide a forward declaration for struct netdev_macsec and not
> have to include that header file.
OK.
> > +
> > struct netpoll_info;
> > struct device;
> > struct phy_device;
> > @@ -1441,6 +1445,10 @@ struct net_device_ops {
> > u32 flags);
> > int (*ndo_xsk_async_xmit)(struct net_device *dev,
> > u32 queue_id);
> > +#ifdef CONFIG_MACSEC
> > + int (*ndo_macsec)(struct net_device *dev,
> > + struct netdev_macsec *macsec);
>
> You would really want to define an API which is more oriented towards
> configuring/deconfiguring a MACsec association here, e.g.: similar to
> what the IPsec offload ndos offer.
This means mostly moving from a single function using a command field to
multiple specialized functions to add/remove each element of MACsec
configuration.
I don't have strong opinion on the single helper vs a structure
containing pointers to specialized ones, but out of curiosity what's the
benefit of such a move? Future additions and maintainability?
> It is not clear to me whether after your patch series we still need to
> create a macsec virtual device, and that gets offloaded onto its real
> device/PHY device, or if we don't need that all?
After this series, we will still need the virtual MACsec interface. When
using hardware offloading this interface isn't doing much, but it's the
interface used to configure all the MACsec connexions.
This is because, and that's specific to MACsec (vs IPsec), a software
implementation is already supported and it's using a virtual interface
to perform all the MACsec related operations (vs hooks in the Rx/Tx
paths). I really wanted to avoid having two interfaces and ways of
configuring MACsec depending on if the offloading is used.
This should also allow in the future to disable at run-time the
offloading on a given interface, and to still have MACsec working in
software (or the opposite, with extra work). For this to work, the
virtual interface still has to provide an Rx and a Tx functions so that
programs can bind onto the same interface, regardless of if the
offloading is enabled.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 3/7] devlink: move devlink health reporter to use devlink msg API
From: Jiri Pirko @ 2019-01-24 9:09 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <f1838db3-e32c-dcf2-5aa6-6756ea508e34@mellanox.com>
Thu, Jan 24, 2019 at 08:45:05AM CET, eranbe@mellanox.com wrote:
>
>
>On 1/23/2019 4:42 PM, Jiri Pirko wrote:
>> Tue, Jan 22, 2019 at 04:57:20PM CET, eranbe@mellanox.com wrote:
>>> Move devlink reporter diagnose and dump operations to use the new msg API.
>>> Redefine the signature of diagnose and dump operations and move the mlx5e
>>> reporter to use it with the new format.
>>>
>>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>>> ---
>>> .../mellanox/mlx5/core/en/reporter_tx.c | 1 +
>>> include/net/devlink.h | 9 +-
>>> net/core/devlink.c | 95 +++++--------------
>>> 3 files changed, 28 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> index fc92850c214a..7238cda670ba 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> @@ -264,6 +264,7 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>>> static const struct devlink_health_reporter_ops mlx5_tx_reporter_ops = {
>>> .name = "TX",
>>> .recover = mlx5e_tx_reporter_recover,
>>> + .diagnose = mlx5e_tx_reporter_diagnose,
>>
>> Unrelated to this patch.
>
>ack.
>
>>
>>
>>> };
>>>
>>> #define MLX5_REPORTER_TX_GRACEFUL_PERIOD 500
>>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>>> index fe323e9b14e1..d66de8b80cc2 100644
>>> --- a/include/net/devlink.h
>>> +++ b/include/net/devlink.h
>>> @@ -442,17 +442,12 @@ struct devlink_health_reporter;
>>>
>>> struct devlink_health_reporter_ops {
>>> char *name;
>>> - unsigned int dump_size;
>>> - unsigned int diagnose_size;
>>> int (*recover)(struct devlink_health_reporter *reporter,
>>> void *priv_ctx);
>>> int (*dump)(struct devlink_health_reporter *reporter,
>>> - struct devlink_health_buffer **buffers_array,
>>> - unsigned int buffer_size, unsigned int num_buffers,
>>> - void *priv_ctx);
>>> + struct devlink_msg_ctx *msg_ctx, void *priv_ctx);
>>> int (*diagnose)(struct devlink_health_reporter *reporter,
>>> - struct devlink_health_buffer **buffers_array,
>>> - unsigned int buffer_size, unsigned int num_buffers);
>>> + struct devlink_msg_ctx *msg_ctx);
>>> };
>>>
>>> struct devlink_ops {
>>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>>> index 57ca096849b3..347b638e6f32 100644
>>> --- a/net/core/devlink.c
>>> +++ b/net/core/devlink.c
>>> @@ -4555,10 +4555,8 @@ static int devlink_msg_snd(struct genl_info *info,
>>>
>>> struct devlink_health_reporter {
>>> struct list_head list;
>>> - struct devlink_health_buffer **dump_buffers_array;
>>> + struct devlink_msg_ctx *dump_msg_ctx;
>>> struct mutex dump_lock; /* lock parallel read/write from dump buffers */
>>> - struct devlink_health_buffer **diagnose_buffers_array;
>>> - struct mutex diagnose_lock; /* lock parallel read/write from diagnose buffers */
>>
>> Howcome you don't need the mutex anymore?
>
>Now, as data is allocated on the fly while diagnose_doit(), no need to
>store the diagnose over the reporter anymore. So no need for any mutex
>locking in order to prepare and send it.
Why the data is allocated on the fly? Shouldn't it be a separate patch
as it looks like a separate change?
>
>>
>>
>>> void *priv;
>>> const struct devlink_health_reporter_ops *ops;
>>> struct devlink *devlink;
>>> @@ -4619,9 +4617,7 @@ devlink_health_reporter_create(struct devlink *devlink,
>>> goto unlock;
>>> }
>>>
>>> - if (WARN_ON(ops->dump && !ops->dump_size) ||
>>> - WARN_ON(ops->diagnose && !ops->diagnose_size) ||
>>> - WARN_ON(auto_recover && !ops->recover) ||
>>> + if (WARN_ON(auto_recover && !ops->recover) ||
>>> WARN_ON(graceful_period && !ops->recover)) {
>>> reporter = ERR_PTR(-EINVAL);
>>> goto unlock;
>>> @@ -4633,31 +4629,8 @@ devlink_health_reporter_create(struct devlink *devlink,
>>> goto unlock;
>>> }
>>>
>>> - if (ops->dump) {
>>> - reporter->dump_buffers_array =
>>> - devlink_health_buffers_create(ops->dump_size);
>>> - if (!reporter->dump_buffers_array) {
>>> - kfree(reporter);
>>> - reporter = ERR_PTR(-ENOMEM);
>>> - goto unlock;
>>> - }
>>> - }
>>> -
>>> - if (ops->diagnose) {
>>> - reporter->diagnose_buffers_array =
>>> - devlink_health_buffers_create(ops->diagnose_size);
>>> - if (!reporter->diagnose_buffers_array) {
>>> - devlink_health_buffers_destroy(reporter->dump_buffers_array,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(ops->dump_size));
>>> - kfree(reporter);
>>> - reporter = ERR_PTR(-ENOMEM);
>>> - goto unlock;
>>> - }
>>> - }
>>> -
>>> list_add_tail(&reporter->list, &devlink->reporter_list);
>>> mutex_init(&reporter->dump_lock);
>>> - mutex_init(&reporter->diagnose_lock);
>>>
>>> reporter->priv = priv;
>>> reporter->ops = ops;
>>> @@ -4680,10 +4653,8 @@ devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
>>> {
>>> mutex_lock(&reporter->devlink->lock);
>>> list_del(&reporter->list);
>>> - devlink_health_buffers_destroy(reporter->dump_buffers_array,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>>> - devlink_health_buffers_destroy(reporter->diagnose_buffers_array,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->diagnose_size));
>>> + if (reporter->dump_msg_ctx)
>>> + devlink_msg_ctx_free(reporter->dump_msg_ctx);
>>> kfree(reporter);
>>> mutex_unlock(&reporter->devlink->lock);
>>> }
>>> @@ -4720,12 +4691,15 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter,
>>> if (reporter->dump_avail)
>>> return 0;
>>>
>>> - devlink_health_buffers_reset(reporter->dump_buffers_array,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>>> - err = reporter->ops->dump(reporter, reporter->dump_buffers_array,
>>> - DEVLINK_HEALTH_BUFFER_SIZE,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size),
>>> - priv_ctx);
>>> + reporter->dump_msg_ctx = devlink_msg_ctx_alloc();
>>> + if (IS_ERR_OR_NULL(reporter->dump_msg_ctx)) {
>>> + err = PTR_ERR(reporter->dump_msg_ctx);
>>> + reporter->dump_msg_ctx = NULL;
>>> + return err;
>>> + }
>>> +
>>> + err = reporter->ops->dump(reporter, reporter->dump_msg_ctx,
>>> + priv_ctx);
>>> if (!err) {
>>> reporter->dump_avail = true;
>>> reporter->dump_ts = jiffies;
>>> @@ -4960,7 +4934,7 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
>>> {
>>> struct devlink *devlink = info->user_ptr[0];
>>> struct devlink_health_reporter *reporter;
>>> - u64 num_of_buffers;
>>> + struct devlink_msg_ctx *msg_ctx;
>>> int err;
>>>
>>> reporter = devlink_health_reporter_get_from_info(devlink, info);
>>> @@ -4970,32 +4944,19 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
>>> if (!reporter->ops->diagnose)
>>> return -EOPNOTSUPP;
>>>
>>> - num_of_buffers =
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->diagnose_size);
>>> + msg_ctx = devlink_msg_ctx_alloc();
>>> + if (IS_ERR_OR_NULL(msg_ctx))
>>> + return PTR_ERR(msg_ctx);
>>>
>>> - mutex_lock(&reporter->diagnose_lock);
>>> - devlink_health_buffers_reset(reporter->diagnose_buffers_array,
>>> - num_of_buffers);
>>> -
>>> - err = reporter->ops->diagnose(reporter,
>>> - reporter->diagnose_buffers_array,
>>> - DEVLINK_HEALTH_BUFFER_SIZE,
>>> - num_of_buffers);
>>> + err = reporter->ops->diagnose(reporter, msg_ctx);
>>
>> So this is not needed to be in reporter now? Why it was needed before?
>
>see reply above.
>
>>
>>
>>
>>> if (err)
>>> goto out;
>>>
>>> - err = devlink_health_buffer_snd(info,
>>> - DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>>> - 0, reporter->diagnose_buffers_array,
>>> - num_of_buffers);
>>> - if (err)
>>> - goto out;
>>> -
>>> - mutex_unlock(&reporter->diagnose_lock);
>>> - return 0;
>>> + err = devlink_msg_snd(info, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>>> + 0, msg_ctx);
>>>
>>> out:
>>> - mutex_unlock(&reporter->diagnose_lock);
>>> + devlink_msg_ctx_free(msg_ctx);
>>> return err;
>>> }
>>>
>>> @@ -5004,8 +4965,8 @@ devlink_health_dump_clear(struct devlink_health_reporter *reporter)
>>> {
>>> reporter->dump_avail = false;
>>> reporter->dump_ts = 0;
>>> - devlink_health_buffers_reset(reporter->dump_buffers_array,
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>>> + devlink_msg_ctx_free(reporter->dump_msg_ctx);
>>> + reporter->dump_msg_ctx = NULL;
>>> }
>>>
>>> static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>>> @@ -5013,7 +4974,6 @@ static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>>> {
>>> struct devlink *devlink = info->user_ptr[0];
>>> struct devlink_health_reporter *reporter;
>>> - u64 num_of_buffers;
>>> int err;
>>>
>>> reporter = devlink_health_reporter_get_from_info(devlink, info);
>>> @@ -5023,18 +4983,13 @@ static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>>> if (!reporter->ops->dump)
>>> return -EOPNOTSUPP;
>>>
>>> - num_of_buffers =
>>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size);
>>> -
>>> mutex_lock(&reporter->dump_lock);
>>> err = devlink_health_do_dump(reporter, NULL);
>>> if (err)
>>> goto out;
>>>
>>> - err = devlink_health_buffer_snd(info,
>>> - DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
>>> - 0, reporter->dump_buffers_array,
>>> - num_of_buffers);
>>> + err = devlink_msg_snd(info, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
>>> + 0, reporter->dump_msg_ctx);
>>>
>>> out:
>>> mutex_unlock(&reporter->dump_lock);
>>> --
>>> 2.17.1
>>>
^ permalink raw reply
* Re: [PATCH net-next 2/7] net/mlx5e: Move driver to use devlink msg API
From: Jiri Pirko @ 2019-01-24 9:08 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <cc2b81fa-d90d-f075-7923-7f4ab92b5ee8@mellanox.com>
Thu, Jan 24, 2019 at 08:39:12AM CET, eranbe@mellanox.com wrote:
>
>
>On 1/23/2019 4:39 PM, Jiri Pirko wrote:
>> Tue, Jan 22, 2019 at 04:57:19PM CET, eranbe@mellanox.com wrote:
>>> As part of the devlink health reporter diagnose ops callback, the mlx5e TX
>>> reporter used devlink health buffers API. Which will soon be depracated.
>>> Modify the reporter to use the new devlink msg API.
>>>
>>> The actual set of the new diagnose function will be done later in the
>>> downstream patch, only when devlink will actually expose a new diagnose
>>> operation that uses the new API.
>>>
>>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>>> ---
>>> .../mellanox/mlx5/core/en/reporter_tx.c | 123 ++++--------------
>>> 1 file changed, 26 insertions(+), 97 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> index d9675afbb924..fc92850c214a 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>>> @@ -192,110 +192,47 @@ static int mlx5e_tx_reporter_recover(struct devlink_health_reporter *reporter,
>>> }
>>>
>>> static int
>>> -mlx5e_tx_reporter_build_diagnose_output(struct devlink_health_buffer *buffer,
>>> +mlx5e_tx_reporter_build_diagnose_output(struct devlink_msg_ctx *msg_ctx,
>>> u32 sqn, u8 state, u8 stopped)
>>
>> What is "state" and "stopped"? Is "stopped" a bool? Is "state" an enum.
>>
>stopped is the reply from netif_xmit_stopped, and it is a bool.
If it is bool, you should add it into message as NLA_FLAG, not NLA_U8
>state is the HW state of the SQ.
>it is defined in the ifc file:
>enum {
> MLX5_SQC_STATE_RST = 0x0,
> MLX5_SQC_STATE_RDY = 0x1,
> MLX5_SQC_STATE_ERR = 0x3,
>};
>I could have translated it into strings, but I though it would be fine
>to leave it as is.
It's fine as u8
>
>>
>>> {
>>> - int err, i;
>>> - int nest = 0;
>>> - char name[20];
>>> -
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT);
>>> - if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> -
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>>> - if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> -
>>> - sprintf(name, "SQ 0x%x", sqn);
>>> - err = devlink_health_buffer_put_object_name(buffer, name);
>>> - if (err)
>>> - goto buffer_error;
>>> -
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>>> - if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> -
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT);
>>> - if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> -
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>>> - if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> -
>>> - err = devlink_health_buffer_put_object_name(buffer, "HW state");
>>> - if (err)
>>> - goto buffer_error;
>>> + int err;
>>>
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>>> + err = devlink_msg_object_start(msg_ctx, "SQ");
>>> if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> + return err;
>>>
>>> - err = devlink_health_buffer_put_value_u8(buffer, state);
>>> + err = devlink_msg_object_start(msg_ctx, NULL);
>>> if (err)
>>> - goto buffer_error;
>>> -
>>> - devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE */
>>> - nest--;
>>> -
>>> - devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR */
>>> - nest--;
>>> + return err;
>>>
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>>> + err = devlink_msg_put_name_value_pair(msg_ctx, "sqn", &sqn,
>>> + sizeof(sqn), NLA_U32);
>>> if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> + return err;
>>>
>>> - err = devlink_health_buffer_put_object_name(buffer, "stopped");
>>> + err = devlink_msg_put_name_value_pair(msg_ctx, "HW state", &state,
>>> + sizeof(state), NLA_U8);
>>> if (err)
>>> - goto buffer_error;
>>> + return err;
>>>
>>> - err = devlink_health_buffer_nest_start(buffer,
>>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>>> + err = devlink_msg_put_name_value_pair(msg_ctx, "stopped", &stopped,
>>> + sizeof(stopped), NLA_U8);
>>> if (err)
>>> - goto buffer_error;
>>> - nest++;
>>> + return err;
>>>
>>> - err = devlink_health_buffer_put_value_u8(buffer, stopped);
>>> + err = devlink_msg_object_end(msg_ctx, NULL);
>>> if (err)
>>> - goto buffer_error;
>>> -
>>> - for (i = 0; i < nest; i++)
>>> - devlink_health_buffer_nest_end(buffer);
>>> -
>>> - return 0;
>>> + return err;
>>>
>>> -buffer_error:
>>> - for (i = 0; i < nest; i++)
>>> - devlink_health_buffer_nest_cancel(buffer);
>>> + err = devlink_msg_object_end(msg_ctx, "SQ");
>>> return err;
>>> }
>>>
>>> static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>>> - struct devlink_health_buffer **buffers_array,
>>> - unsigned int buffer_size,
>>> - unsigned int num_buffers)
>>> + struct devlink_msg_ctx *msg_ctx)
>>> {
>>> struct mlx5e_priv *priv = devlink_health_reporter_priv(reporter);
>>> - unsigned int buff = 0;
>>> - int i = 0, err = 0;
>>> -
>>> - if (buffer_size < MLX5E_TX_REPORTER_PER_SQ_MAX_LEN)
>>> - return -ENOMEM;
>>> + int i, err = 0;
>>>
>>> mutex_lock(&priv->state_lock);
>>>
>>> @@ -304,7 +241,8 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>>> return 0;
>>> }
>>>
>>> - while (i < priv->channels.num * priv->channels.params.num_tc) {
>>> + for (i = 0; i < priv->channels.num * priv->channels.params.num_tc;
>>> + i++) {
>>> struct mlx5e_txqsq *sq = priv->txq2sq[i];
>>> u8 state;
>>>
>>> @@ -312,15 +250,11 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>>> if (err)
>>> break;
>>>
>>> - err = mlx5e_tx_reporter_build_diagnose_output(buffers_array[buff],
>>> - sq->sqn, state,
>>> + err = mlx5e_tx_reporter_build_diagnose_output(msg_ctx, sq->sqn,
>>> + state,
>>> netif_xmit_stopped(sq->txq));
>>
>> This should be an array. On SQ entry : one member of array.
>>
>> So if you want to change it, you need to do this in 2 patches. One API,
>> one the actual layout. :/
>>
>>
>>
>>> - if (err) {
>>> - if (++buff == num_buffers)
>>> - break;
>>> - } else {
>>> - i++;
>>> - }
>>> + if (err)
>>> + break;
>>> }
>>>
>>> mutex_unlock(&priv->state_lock);
>>> @@ -330,11 +264,6 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>>> static const struct devlink_health_reporter_ops mlx5_tx_reporter_ops = {
>>> .name = "TX",
>>> .recover = mlx5e_tx_reporter_recover,
>>> - .diagnose_size = MLX5E_MAX_NUM_CHANNELS * MLX5E_MAX_NUM_TC *
>>> - MLX5E_TX_REPORTER_PER_SQ_MAX_LEN,
>>> - .diagnose = mlx5e_tx_reporter_diagnose,
>>
>> So you change the callback, remove it so the dissection is broken.
>
>This is needed in order to have this patch compiled.
You have to figure it out without breakage. This is not the way to
do this.
>
>>
>>
>>
>>> - .dump_size = 0,
>>> - .dump = NULL,
>>
>>
>> This has 0 relation to the patch. Should be separate.
>
>ack.
>
>>
>>
>>> };
>>>
>>> #define MLX5_REPORTER_TX_GRACEFUL_PERIOD 500
>>> --
>>> 2.17.1
>>>
^ permalink raw reply
* Re: [PATCH 2/4] can: bcm: check timer values before ktime conversion
From: Oliver Hartkopp @ 2019-01-24 9:13 UTC (permalink / raw)
To: Sasha Levin, Marc Kleine-Budde, netdev; +Cc: davem, linux-can, stable
In-Reply-To: <20190123225748.AF445218A1@mail.kernel.org>
Hi Shasha,
On 23.01.19 23:57, Sasha Levin wrote:
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: 2.6.26+
>
> The bot has tested the following trees: v4.20.3, v4.19.16, v4.14.94, v4.9.151, v4.4.171, v3.18.132.
>
> v4.20.3: Build OK!
> v4.19.16: Build OK!
> v4.14.94: Build OK!
> v4.9.151: Build OK!
> v4.4.171: Failed to apply! Possible dependencies:
> 2b5f5f5dc114 ("can: bcm: unify bcm_msg_head handling and prepare function parameters")
> 6f3b911d5f29 ("can: bcm: add support for CAN FD frames")
> 72c8a89ad2e4 ("can: bcm: use CAN frame instead of can_frame in comments")
> 95acb490ec51 ("can: bcm: fix indention and other minor style issues")
>
> v3.18.132: Failed to apply! Possible dependencies:
> 069f8457ae52 ("can: fix spelling errors")
> 2b5f5f5dc114 ("can: bcm: unify bcm_msg_head handling and prepare function parameters")
> 6ce8e9ce5989 ("new helper: memcpy_from_msg()")
> 6f3b911d5f29 ("can: bcm: add support for CAN FD frames")
> 72c8a89ad2e4 ("can: bcm: use CAN frame instead of can_frame in comments")
> 95acb490ec51 ("can: bcm: fix indention and other minor style issues")
> ba61a8d9d780 ("can: avoid using timeval for uapi")
>
>
> How should we proceed with this patch?
As we do have a proper upstream commit hash now ...
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git/commit/?h=testing&id=93171ba6f1deffd82f381d36cb13177872d023f6
... here is the ported patch for kernels pre version 4.8:
https://marc.info/?l=linux-can&m=154832094402622&w=2
Thanks,
Oliver
ps. the pull request to Dave is still hanging here:
https://marc.info/?l=linux-can&m=154831478900596&w=2
^ permalink raw reply
* [PATCH] [stable pre-4.8] can: bcm: check timer values before ktime conversion
From: Oliver Hartkopp @ 2019-01-24 9:08 UTC (permalink / raw)
To: davem, netdev, stable
Cc: linux-can, lifeasageek, threeearcat, syzkaller, nautsch2,
Oliver Hartkopp, Kyungtae Kim, Marc Kleine-Budde
Kyungtae Kim detected a potential integer overflow in bcm_[rx|tx]_setup()
when the conversion into ktime multiplies the given value with NSEC_PER_USEC
(1000).
Reference: https://marc.info/?l=linux-can&m=154732118819828&w=2
Add a check for the given tv_usec, so that the value stays below one second.
Additionally limit the tv_sec value to a reasonable value for CAN related
use-cases of 400 days and ensure all values to be positive.
This patch is the pre-4.8 version of upstream commit 93171ba6f1deffd8
Reported-by: Kyungtae Kim <kt0755@gmail.com>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org> # versions 2.6.26 to 4.7
Tested-by: Kyungtae Kim <kt0755@gmail.com>
Acked-by: Andre Naujoks <nautsch2@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 6863310d6973..01d489d0a3de 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -67,6 +67,9 @@
*/
#define MAX_NFRAMES 256
+/* limit timers to 400 days for sending/timeouts */
+#define BCM_TIMER_SEC_MAX (400 * 24 * 60 * 60)
+
/* use of last_frames[index].can_dlc */
#define RX_RECV 0x40 /* received data for this element */
#define RX_THR 0x80 /* element not been sent due to throttle feature */
@@ -136,6 +139,22 @@ static inline ktime_t bcm_timeval_to_ktime(struct bcm_timeval tv)
return ktime_set(tv.tv_sec, tv.tv_usec * NSEC_PER_USEC);
}
+/* check limitations for timeval provided by user */
+static bool bcm_is_invalid_tv(struct bcm_msg_head *msg_head)
+{
+ if ((msg_head->ival1.tv_sec < 0) ||
+ (msg_head->ival1.tv_sec > BCM_TIMER_SEC_MAX) ||
+ (msg_head->ival1.tv_usec < 0) ||
+ (msg_head->ival1.tv_usec >= USEC_PER_SEC) ||
+ (msg_head->ival2.tv_sec < 0) ||
+ (msg_head->ival2.tv_sec > BCM_TIMER_SEC_MAX) ||
+ (msg_head->ival2.tv_usec < 0) ||
+ (msg_head->ival2.tv_usec >= USEC_PER_SEC))
+ return true;
+
+ return false;
+}
+
#define CFSIZ sizeof(struct can_frame)
#define OPSIZ sizeof(struct bcm_op)
#define MHSIZ sizeof(struct bcm_msg_head)
@@ -846,6 +865,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (msg_head->nframes < 1 || msg_head->nframes > MAX_NFRAMES)
return -EINVAL;
+ /* check timeval limitations */
+ if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
+ return -EINVAL;
+
/* check the given can_id */
op = bcm_find_op(&bo->tx_ops, msg_head->can_id, ifindex);
@@ -1011,6 +1034,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
(!(msg_head->can_id & CAN_RTR_FLAG))))
return -EINVAL;
+ /* check timeval limitations */
+ if ((msg_head->flags & SETTIMER) && bcm_is_invalid_tv(msg_head))
+ return -EINVAL;
+
/* check the given can_id */
op = bcm_find_op(&bo->rx_ops, msg_head->can_id, ifindex);
if (op) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next] bpftool: feature probing, change default action
From: Quentin Monnet @ 2019-01-24 9:05 UTC (permalink / raw)
To: Prashant Bhole, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, Jakub Kicinski
In-Reply-To: <20190124025157.6812-1-bhole_prashant_q7@lab.ntt.co.jp>
2019-01-24 11:51 UTC+0900 ~ Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> When 'bpftool feature' is executed it shows incorrect help string.
>
> test# bpftool feature
> Usage: bpftool bpftool probe [COMPONENT] [macros [prefix PREFIX]]
> bpftool bpftool help
>
> COMPONENT := { kernel | dev NAME }
>
> Instead of fixing the help text by tweaking argv[] indices, this
> patch changes the default action to 'probe'. It makes the behavior
> consistent with other subcommands, where first subcommand without
> extra parameter results in 'show' action.
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> tools/bpf/bpftool/feature.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 993c6f1e5473..d672d9086fff 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -753,8 +753,8 @@ static int do_help(int argc, char **argv)
> }
>
> static const struct cmd cmds[] = {
> - { "help", do_help },
> { "probe", do_probe },
> + { "help", do_help },
> { 0 }
> };
>
>
Oh, thanks! This seems to be the correct thing to do indeed.
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
^ permalink raw reply
* Re: [PATCH net-next 03/10] net: macsec: move some definitions in a dedicated header
From: Antoine Tenart @ 2019-01-24 8:58 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, sd, andrew, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <955d1bdf-037e-ae26-a9da-8da2a51a7cbd@gmail.com>
Hi Florian,
On Wed, Jan 23, 2019 at 12:11:37PM -0800, Florian Fainelli wrote:
> On 1/23/19 7:56 AM, Antoine Tenart wrote:
> > This patch moves some structure, type and identifier definitions into a
> > MACsec specific header. This patch does not modify how the MACsec code
> > is running and only move things around. This is a preparation for the
> > future MACsec hardware offloading support, which will re-use those
> > definitions outside macsec.c.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> > ---
> > drivers/net/macsec.c | 164 +--------------------------------------
> > include/net/macsec.h | 178 +++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 179 insertions(+), 163 deletions(-)
> > create mode 100644 include/net/macsec.h
> >
> > diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> > index 56e354305f76..c3a138dd4386 100644
> > --- a/drivers/net/macsec.c
> > +++ b/drivers/net/macsec.c
> > @@ -17,10 +17,9 @@
> > #include <net/sock.h>
> > #include <net/gro_cells.h>
> >
> > +#include <net/macsec.h>
> > #include <uapi/linux/if_macsec.h>
>
> I would probably go with include/linux/if_macsec.h and have
> uapi/linux/if_macsec.h include that file directly. This would be
> consistent with other types of network interfaces: bridge, vlan etc.
It's always a good idea to stay consistent with what's already done.
I'll update in v2.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 05/10] net: phy: introduce a phy_driver macsec helper
From: Antoine Tenart @ 2019-01-24 8:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, sd, f.fainelli, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <20190123170816.GH5310@lunn.ch>
Hi Andrew,
On Wed, Jan 23, 2019 at 06:08:16PM +0100, Andrew Lunn wrote:
> > +int phy_macsec(struct phy_device *phydev, struct netdev_macsec *macsec)
> > +{
> > + int ret = -EOPNOTSUPP;
> > +
> > + if (!phydev->drv)
> > + return -EIO;
> > +
> > + mutex_lock(&phydev->lock);
> > +
> > + if (phydev->drv->macsec)
> > + ret = phydev->drv->macsec(phydev, macsec);
> > +
> > + mutex_unlock(&phydev->lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(phy_macsec);
> > +
>
> > @@ -630,6 +634,10 @@ struct phy_driver {
> > struct ethtool_tunable *tuna,
> > const void *data);
> > int (*set_loopback)(struct phy_device *dev, bool enable);
> > +
> > +#ifdef CONFIG_MACSEC
> > + int (*macsec)(struct phy_device *dev, struct netdev_macsec *macsec);
> > +#endif
>
>
> So the member only exists if CONFIG_MACSEC is defined. So i think you
> need similar protection in phy_macsec() or you are going to try to
> access a member which sometimes does not exist.
Right, I'll fix that and protect the phy_macsec definition within an
'#ifdef CONFIG_MACSEC'.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH bpf-next v5 4/9] tools: bpftool: add probes for eBPF program types
From: Quentin Monnet @ 2019-01-24 8:55 UTC (permalink / raw)
To: Andrey Ignatov
Cc: Alexei Starovoitov, Daniel Borkmann, netdev@vger.kernel.org,
oss-drivers@netronome.com, Arnaldo Carvalho de Melo,
Jesper Dangaard Brouer, Stanislav Fomichev
In-Reply-To: <20190123234429.GA8297@rdna-mbp.dhcp.thefacebook.com>
2019-01-23 23:44 UTC+0000 ~ Andrey Ignatov <rdna@fb.com>
> Quentin Monnet <quentin.monnet@netronome.com> [Thu, 2019-01-17 07:28 -0800]:
>> Introduce probes for supported BPF program types in libbpf, and call it
>> from bpftool to test what types are available on the system. The probe
>> simply consists in loading a very basic program of that type and see if
>> the verifier complains or not.
>>
>> Sample output:
>>
>> # bpftool feature probe kernel
>> ...
>> Scanning eBPF program types...
>> eBPF program_type socket_filter is available
>> eBPF program_type kprobe is available
>> eBPF program_type sched_cls is available
>> ...
>>
>> # bpftool --json --pretty feature probe kernel
>> {
>> ...
>> "program_types": {
>> "have_socket_filter_prog_type": true,
>> "have_kprobe_prog_type": true,
>> "have_sched_cls_prog_type": true,
>> ...
>> }
>> }
>
> Hi Quentin,
>
> Awesome work! I wondef if you have plan to add similar functionality for
> attach types?
>
> For example BPF_CGROUP_INET{4,6}_CONNECT were added in 4.17 but
> BPF_CGROUP_UDP{4,6}_SENDMSG later in 4.18 even though they correspond to
> same prog type (BPF_PROG_TYPE_CGROUP_SOCK_ADDR).
Hi Andrey, thanks! And yeah adding probes for the attach types would be
one of the next logical steps, although to be honest it is not at the
top of my to-do list right now. I'll probably add them eventually, but
if you have a need for such probes today feel free to give it a go!
Best regards,
Quentin
^ permalink raw reply
* Re: [PATCH net-next 02/10] net: macsec: convert to SPDX
From: Antoine Tenart @ 2019-01-24 8:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, sd, f.fainelli, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <20190123170306.GG5310@lunn.ch>
Hi Andrew,
On Wed, Jan 23, 2019 at 06:03:06PM +0100, Andrew Lunn wrote:
> On Wed, Jan 23, 2019 at 04:56:30PM +0100, Antoine Tenart wrote:
> > This cosmetic patch converts the macsec implementation license header to
> > the now recommended format, SPDX.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
> > ---
> > drivers/net/macsec.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> > index 64a982563d59..56e354305f76 100644
> > --- a/drivers/net/macsec.c
> > +++ b/drivers/net/macsec.c
> > @@ -1,12 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > /*
> > * drivers/net/macsec.c - MACsec device
> > *
> > * Copyright (c) 2015 Sabrina Dubroca <sd@queasysnail.net>
> > - *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > */
> >
> > #include <linux/types.h>
>
> linux$ tail -4 drivers/net/macsec.c
> MODULE_ALIAS_GENL_FAMILY("macsec");
>
> MODULE_DESCRIPTION("MACsec IEEE 802.1AE");
> MODULE_LICENSE("GPL v2");
>
> The license text and the MODULE_LICENSE() don't agree with each other.
> It would be good to fix this as well.
That's right, I'll fix it in v2. In such cases, the license text takes
precedence over the MODULE_LICENSE() definition ?
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches
From: Greg KH @ 2019-01-24 8:10 UTC (permalink / raw)
To: Kees Cook
Cc: Jani Nikula, Edwin Zimmerman, dev, Ard Biesheuvel,
Network Development, intel-gfx, linux-usb, LKML,
Maling list - DRI developers, Linux-MM, linux-security-module,
Kernel Hardening, intel-wired-lan, linux-fsdevel@vger.kernel.org,
xen-devel, Laura Abbott, linux-kbuild, Alexander Popov
In-Reply-To: <CAGXu5jJUxHtFq0rBJ9FwzMcZDWnusPUauC_=MaOz7H0_PF25jQ@mail.gmail.com>
On Thu, Jan 24, 2019 at 07:55:51AM +1300, Kees Cook wrote:
> On Thu, Jan 24, 2019 at 4:44 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Wed, 23 Jan 2019, Edwin Zimmerman <edwin@211mainstreet.net> wrote:
> > > On Wed, 23 Jan 2019, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >> On Wed, 23 Jan 2019, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > >> >> Variables declared in a switch statement before any case statements
> > >> >> cannot be initialized, so move all instances out of the switches.
> > >> >> After this, future always-initialized stack variables will work
> > >> >> and not throw warnings like this:
> > >> >>
> > >> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
> > >> >> fs/fcntl.c:738:13: warning: statement will never be executed [-Wswitch-unreachable]
> > >> >> siginfo_t si;
> > >> >> ^~
> > >> >
> > >> > That's a pain, so this means we can't have any new variables in { }
> > >> > scope except for at the top of a function?
>
> Just in case this wasn't clear: no, it's just the switch statement
> before the first "case". I cannot imagine how bad it would be if we
> couldn't have block-scoped variables! Heh. :)
Sorry, it was not clear at first glance. So no more objection from me
for this change.
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v1] l2tp: fix reading optional fields
From: Jacob Wen @ 2019-01-24 7:51 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: jchapman
In-Reply-To: <ea5c5e46-568b-2fe2-d1ab-f1131240ab4e@gmail.com>
On 1/23/19 10:51 AM, Eric Dumazet wrote:
>
> On 01/22/2019 06:30 PM, Jacob Wen wrote:
>> Use pskb_may_pull() to make sure the optional fields are in skb linear
>> parts.
>>
>> Signed-off-by: Jacob Wen <jian.w.wen@oracle.com>
>> ---
>> v1: fix warning: ISO C90 forbids mixed declarations and code
>> ---
>> net/l2tp/l2tp_core.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
>> index 26f1d435696a..c0dfa2bcd218 100644
>> --- a/net/l2tp/l2tp_core.c
>> +++ b/net/l2tp/l2tp_core.c
>> @@ -627,6 +627,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
>>
>> /* Parse and check optional cookie */
>> if (session->peer_cookie_len > 0) {
>> + if (!pskb_may_pull(skb, ptr - optr + session->peer_cookie_len))
>> + goto discard;
> This is completely buggy.
>
> After pskb_may_pull(), the ptr and optr pointers might point to freed data.
>
> So the memcmp() might crash hard.
Fixed in v2. Thanks.
>
>> if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
>> l2tp_info(tunnel, L2TP_MSG_DATA,
>> "%s: cookie mismatch (%u/%u). Discarding.\n",
>> @@ -649,6 +651,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
>> L2TP_SKB_CB(skb)->has_seq = 0;
>> if (tunnel->version == L2TP_HDR_VER_2) {
>> if (hdrflags & L2TP_HDRFLAG_S) {
>> + if (!pskb_may_pull(skb, ptr - optr + 4))
>> + goto discard;
>> ns = ntohs(*(__be16 *) ptr);
>> ptr += 2;
>> nr = ntohs(*(__be16 *) ptr);
>> @@ -663,8 +667,12 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
>> session->name, ns, nr, session->nr);
>> }
>> } else if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
>> - u32 l2h = ntohl(*(__be32 *) ptr);
>> + u32 l2h;
>> +
>> + if (!pskb_may_pull(skb, ptr - optr + 4))
>> + goto discard;
>>
>> + l2h = ntohl(*(__be32 *) ptr);
>> if (l2h & 0x40000000) {
>> ns = l2h & 0x00ffffff;
>>
>> @@ -729,6 +737,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
>> if (tunnel->version == L2TP_HDR_VER_2) {
>> /* If offset bit set, skip it. */
>> if (hdrflags & L2TP_HDRFLAG_O) {
>> + if (!pskb_may_pull(skb, ptr - optr + 2))
>> + goto discard;
>> offset = ntohs(*(__be16 *)ptr);
>> ptr += 2 + offset;
>> }
>>
--
Jacob
^ permalink raw reply
* [PATCH net-next v2] net: l2tp: fix reading optional fields of L2TPv3
From: Jacob Wen @ 2019-01-24 7:49 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, g.nault
Use pskb_may_pull() to make sure the optional fields are in skb linear
parts, so we can safely read them later.
It's easy to reproduce the issue with a net driver that supports paged
skb data. Just create a L2TPv3 over IP tunnel and then generates some
network traffic.
Once reproduced, rx err in /sys/kernel/debug/l2tp/tunnels will increase.
Signed-off-by: Jacob Wen <jian.w.wen@oracle.com>
---
Changes in v2:
1. Only fix L2TPv3 to make code simple.
To fix both L2TPv3 and L2TPv2, we'd better refactor l2tp_recv_common.
It's complicated to do so.
2. Reloading pointers after pskb_may_pull
---
net/l2tp/l2tp_core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 26f1d435696a..e9a17c634c1a 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -625,6 +625,20 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
int offset;
u32 ns, nr;
+ if (tunnel->version != L2TP_HDR_VER_2) {
+ int opt_len = session->peer_cookie_len +
+ l2tp_get_l2specific_len(session);
+
+ if (opt_len > 0) {
+ int off = ptr - optr;
+
+ if (!pskb_may_pull(skb, off + opt_len))
+ goto discard;
+ optr = skb->data;
+ ptr = optr + off;
+ }
+ }
+
/* Parse and check optional cookie */
if (session->peer_cookie_len > 0) {
if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 3/7] devlink: move devlink health reporter to use devlink msg API
From: Eran Ben Elisha @ 2019-01-24 7:45 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <20190123144228.GI2191@nanopsycho>
On 1/23/2019 4:42 PM, Jiri Pirko wrote:
> Tue, Jan 22, 2019 at 04:57:20PM CET, eranbe@mellanox.com wrote:
>> Move devlink reporter diagnose and dump operations to use the new msg API.
>> Redefine the signature of diagnose and dump operations and move the mlx5e
>> reporter to use it with the new format.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> .../mellanox/mlx5/core/en/reporter_tx.c | 1 +
>> include/net/devlink.h | 9 +-
>> net/core/devlink.c | 95 +++++--------------
>> 3 files changed, 28 insertions(+), 77 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> index fc92850c214a..7238cda670ba 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> @@ -264,6 +264,7 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>> static const struct devlink_health_reporter_ops mlx5_tx_reporter_ops = {
>> .name = "TX",
>> .recover = mlx5e_tx_reporter_recover,
>> + .diagnose = mlx5e_tx_reporter_diagnose,
>
> Unrelated to this patch.
ack.
>
>
>> };
>>
>> #define MLX5_REPORTER_TX_GRACEFUL_PERIOD 500
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index fe323e9b14e1..d66de8b80cc2 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -442,17 +442,12 @@ struct devlink_health_reporter;
>>
>> struct devlink_health_reporter_ops {
>> char *name;
>> - unsigned int dump_size;
>> - unsigned int diagnose_size;
>> int (*recover)(struct devlink_health_reporter *reporter,
>> void *priv_ctx);
>> int (*dump)(struct devlink_health_reporter *reporter,
>> - struct devlink_health_buffer **buffers_array,
>> - unsigned int buffer_size, unsigned int num_buffers,
>> - void *priv_ctx);
>> + struct devlink_msg_ctx *msg_ctx, void *priv_ctx);
>> int (*diagnose)(struct devlink_health_reporter *reporter,
>> - struct devlink_health_buffer **buffers_array,
>> - unsigned int buffer_size, unsigned int num_buffers);
>> + struct devlink_msg_ctx *msg_ctx);
>> };
>>
>> struct devlink_ops {
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 57ca096849b3..347b638e6f32 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -4555,10 +4555,8 @@ static int devlink_msg_snd(struct genl_info *info,
>>
>> struct devlink_health_reporter {
>> struct list_head list;
>> - struct devlink_health_buffer **dump_buffers_array;
>> + struct devlink_msg_ctx *dump_msg_ctx;
>> struct mutex dump_lock; /* lock parallel read/write from dump buffers */
>> - struct devlink_health_buffer **diagnose_buffers_array;
>> - struct mutex diagnose_lock; /* lock parallel read/write from diagnose buffers */
>
> Howcome you don't need the mutex anymore?
Now, as data is allocated on the fly while diagnose_doit(), no need to
store the diagnose over the reporter anymore. So no need for any mutex
locking in order to prepare and send it.
>
>
>> void *priv;
>> const struct devlink_health_reporter_ops *ops;
>> struct devlink *devlink;
>> @@ -4619,9 +4617,7 @@ devlink_health_reporter_create(struct devlink *devlink,
>> goto unlock;
>> }
>>
>> - if (WARN_ON(ops->dump && !ops->dump_size) ||
>> - WARN_ON(ops->diagnose && !ops->diagnose_size) ||
>> - WARN_ON(auto_recover && !ops->recover) ||
>> + if (WARN_ON(auto_recover && !ops->recover) ||
>> WARN_ON(graceful_period && !ops->recover)) {
>> reporter = ERR_PTR(-EINVAL);
>> goto unlock;
>> @@ -4633,31 +4629,8 @@ devlink_health_reporter_create(struct devlink *devlink,
>> goto unlock;
>> }
>>
>> - if (ops->dump) {
>> - reporter->dump_buffers_array =
>> - devlink_health_buffers_create(ops->dump_size);
>> - if (!reporter->dump_buffers_array) {
>> - kfree(reporter);
>> - reporter = ERR_PTR(-ENOMEM);
>> - goto unlock;
>> - }
>> - }
>> -
>> - if (ops->diagnose) {
>> - reporter->diagnose_buffers_array =
>> - devlink_health_buffers_create(ops->diagnose_size);
>> - if (!reporter->diagnose_buffers_array) {
>> - devlink_health_buffers_destroy(reporter->dump_buffers_array,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(ops->dump_size));
>> - kfree(reporter);
>> - reporter = ERR_PTR(-ENOMEM);
>> - goto unlock;
>> - }
>> - }
>> -
>> list_add_tail(&reporter->list, &devlink->reporter_list);
>> mutex_init(&reporter->dump_lock);
>> - mutex_init(&reporter->diagnose_lock);
>>
>> reporter->priv = priv;
>> reporter->ops = ops;
>> @@ -4680,10 +4653,8 @@ devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
>> {
>> mutex_lock(&reporter->devlink->lock);
>> list_del(&reporter->list);
>> - devlink_health_buffers_destroy(reporter->dump_buffers_array,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>> - devlink_health_buffers_destroy(reporter->diagnose_buffers_array,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->diagnose_size));
>> + if (reporter->dump_msg_ctx)
>> + devlink_msg_ctx_free(reporter->dump_msg_ctx);
>> kfree(reporter);
>> mutex_unlock(&reporter->devlink->lock);
>> }
>> @@ -4720,12 +4691,15 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter,
>> if (reporter->dump_avail)
>> return 0;
>>
>> - devlink_health_buffers_reset(reporter->dump_buffers_array,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>> - err = reporter->ops->dump(reporter, reporter->dump_buffers_array,
>> - DEVLINK_HEALTH_BUFFER_SIZE,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size),
>> - priv_ctx);
>> + reporter->dump_msg_ctx = devlink_msg_ctx_alloc();
>> + if (IS_ERR_OR_NULL(reporter->dump_msg_ctx)) {
>> + err = PTR_ERR(reporter->dump_msg_ctx);
>> + reporter->dump_msg_ctx = NULL;
>> + return err;
>> + }
>> +
>> + err = reporter->ops->dump(reporter, reporter->dump_msg_ctx,
>> + priv_ctx);
>> if (!err) {
>> reporter->dump_avail = true;
>> reporter->dump_ts = jiffies;
>> @@ -4960,7 +4934,7 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
>> {
>> struct devlink *devlink = info->user_ptr[0];
>> struct devlink_health_reporter *reporter;
>> - u64 num_of_buffers;
>> + struct devlink_msg_ctx *msg_ctx;
>> int err;
>>
>> reporter = devlink_health_reporter_get_from_info(devlink, info);
>> @@ -4970,32 +4944,19 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
>> if (!reporter->ops->diagnose)
>> return -EOPNOTSUPP;
>>
>> - num_of_buffers =
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->diagnose_size);
>> + msg_ctx = devlink_msg_ctx_alloc();
>> + if (IS_ERR_OR_NULL(msg_ctx))
>> + return PTR_ERR(msg_ctx);
>>
>> - mutex_lock(&reporter->diagnose_lock);
>> - devlink_health_buffers_reset(reporter->diagnose_buffers_array,
>> - num_of_buffers);
>> -
>> - err = reporter->ops->diagnose(reporter,
>> - reporter->diagnose_buffers_array,
>> - DEVLINK_HEALTH_BUFFER_SIZE,
>> - num_of_buffers);
>> + err = reporter->ops->diagnose(reporter, msg_ctx);
>
> So this is not needed to be in reporter now? Why it was needed before?
see reply above.
>
>
>
>> if (err)
>> goto out;
>>
>> - err = devlink_health_buffer_snd(info,
>> - DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>> - 0, reporter->diagnose_buffers_array,
>> - num_of_buffers);
>> - if (err)
>> - goto out;
>> -
>> - mutex_unlock(&reporter->diagnose_lock);
>> - return 0;
>> + err = devlink_msg_snd(info, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
>> + 0, msg_ctx);
>>
>> out:
>> - mutex_unlock(&reporter->diagnose_lock);
>> + devlink_msg_ctx_free(msg_ctx);
>> return err;
>> }
>>
>> @@ -5004,8 +4965,8 @@ devlink_health_dump_clear(struct devlink_health_reporter *reporter)
>> {
>> reporter->dump_avail = false;
>> reporter->dump_ts = 0;
>> - devlink_health_buffers_reset(reporter->dump_buffers_array,
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size));
>> + devlink_msg_ctx_free(reporter->dump_msg_ctx);
>> + reporter->dump_msg_ctx = NULL;
>> }
>>
>> static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>> @@ -5013,7 +4974,6 @@ static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>> {
>> struct devlink *devlink = info->user_ptr[0];
>> struct devlink_health_reporter *reporter;
>> - u64 num_of_buffers;
>> int err;
>>
>> reporter = devlink_health_reporter_get_from_info(devlink, info);
>> @@ -5023,18 +4983,13 @@ static int devlink_nl_cmd_health_reporter_dump_get_doit(struct sk_buff *skb,
>> if (!reporter->ops->dump)
>> return -EOPNOTSUPP;
>>
>> - num_of_buffers =
>> - DEVLINK_HEALTH_SIZE_TO_BUFFERS(reporter->ops->dump_size);
>> -
>> mutex_lock(&reporter->dump_lock);
>> err = devlink_health_do_dump(reporter, NULL);
>> if (err)
>> goto out;
>>
>> - err = devlink_health_buffer_snd(info,
>> - DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
>> - 0, reporter->dump_buffers_array,
>> - num_of_buffers);
>> + err = devlink_msg_snd(info, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
>> + 0, reporter->dump_msg_ctx);
>>
>> out:
>> mutex_unlock(&reporter->dump_lock);
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH net-next 2/7] net/mlx5e: Move driver to use devlink msg API
From: Eran Ben Elisha @ 2019-01-24 7:39 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <20190123143930.GH2191@nanopsycho>
On 1/23/2019 4:39 PM, Jiri Pirko wrote:
> Tue, Jan 22, 2019 at 04:57:19PM CET, eranbe@mellanox.com wrote:
>> As part of the devlink health reporter diagnose ops callback, the mlx5e TX
>> reporter used devlink health buffers API. Which will soon be depracated.
>> Modify the reporter to use the new devlink msg API.
>>
>> The actual set of the new diagnose function will be done later in the
>> downstream patch, only when devlink will actually expose a new diagnose
>> operation that uses the new API.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> .../mellanox/mlx5/core/en/reporter_tx.c | 123 ++++--------------
>> 1 file changed, 26 insertions(+), 97 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> index d9675afbb924..fc92850c214a 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
>> @@ -192,110 +192,47 @@ static int mlx5e_tx_reporter_recover(struct devlink_health_reporter *reporter,
>> }
>>
>> static int
>> -mlx5e_tx_reporter_build_diagnose_output(struct devlink_health_buffer *buffer,
>> +mlx5e_tx_reporter_build_diagnose_output(struct devlink_msg_ctx *msg_ctx,
>> u32 sqn, u8 state, u8 stopped)
>
> What is "state" and "stopped"? Is "stopped" a bool? Is "state" an enum.
>
stopped is the reply from netif_xmit_stopped, and it is a bool.
state is the HW state of the SQ.
it is defined in the ifc file:
enum {
MLX5_SQC_STATE_RST = 0x0,
MLX5_SQC_STATE_RDY = 0x1,
MLX5_SQC_STATE_ERR = 0x3,
};
I could have translated it into strings, but I though it would be fine
to leave it as is.
>
>> {
>> - int err, i;
>> - int nest = 0;
>> - char name[20];
>> -
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT);
>> - if (err)
>> - goto buffer_error;
>> - nest++;
>> -
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>> - if (err)
>> - goto buffer_error;
>> - nest++;
>> -
>> - sprintf(name, "SQ 0x%x", sqn);
>> - err = devlink_health_buffer_put_object_name(buffer, name);
>> - if (err)
>> - goto buffer_error;
>> -
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>> - if (err)
>> - goto buffer_error;
>> - nest++;
>> -
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT);
>> - if (err)
>> - goto buffer_error;
>> - nest++;
>> -
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>> - if (err)
>> - goto buffer_error;
>> - nest++;
>> -
>> - err = devlink_health_buffer_put_object_name(buffer, "HW state");
>> - if (err)
>> - goto buffer_error;
>> + int err;
>>
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>> + err = devlink_msg_object_start(msg_ctx, "SQ");
>> if (err)
>> - goto buffer_error;
>> - nest++;
>> + return err;
>>
>> - err = devlink_health_buffer_put_value_u8(buffer, state);
>> + err = devlink_msg_object_start(msg_ctx, NULL);
>> if (err)
>> - goto buffer_error;
>> -
>> - devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE */
>> - nest--;
>> -
>> - devlink_health_buffer_nest_end(buffer); /* DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR */
>> - nest--;
>> + return err;
>>
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_PAIR);
>> + err = devlink_msg_put_name_value_pair(msg_ctx, "sqn", &sqn,
>> + sizeof(sqn), NLA_U32);
>> if (err)
>> - goto buffer_error;
>> - nest++;
>> + return err;
>>
>> - err = devlink_health_buffer_put_object_name(buffer, "stopped");
>> + err = devlink_msg_put_name_value_pair(msg_ctx, "HW state", &state,
>> + sizeof(state), NLA_U8);
>> if (err)
>> - goto buffer_error;
>> + return err;
>>
>> - err = devlink_health_buffer_nest_start(buffer,
>> - DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE);
>> + err = devlink_msg_put_name_value_pair(msg_ctx, "stopped", &stopped,
>> + sizeof(stopped), NLA_U8);
>> if (err)
>> - goto buffer_error;
>> - nest++;
>> + return err;
>>
>> - err = devlink_health_buffer_put_value_u8(buffer, stopped);
>> + err = devlink_msg_object_end(msg_ctx, NULL);
>> if (err)
>> - goto buffer_error;
>> -
>> - for (i = 0; i < nest; i++)
>> - devlink_health_buffer_nest_end(buffer);
>> -
>> - return 0;
>> + return err;
>>
>> -buffer_error:
>> - for (i = 0; i < nest; i++)
>> - devlink_health_buffer_nest_cancel(buffer);
>> + err = devlink_msg_object_end(msg_ctx, "SQ");
>> return err;
>> }
>>
>> static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>> - struct devlink_health_buffer **buffers_array,
>> - unsigned int buffer_size,
>> - unsigned int num_buffers)
>> + struct devlink_msg_ctx *msg_ctx)
>> {
>> struct mlx5e_priv *priv = devlink_health_reporter_priv(reporter);
>> - unsigned int buff = 0;
>> - int i = 0, err = 0;
>> -
>> - if (buffer_size < MLX5E_TX_REPORTER_PER_SQ_MAX_LEN)
>> - return -ENOMEM;
>> + int i, err = 0;
>>
>> mutex_lock(&priv->state_lock);
>>
>> @@ -304,7 +241,8 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>> return 0;
>> }
>>
>> - while (i < priv->channels.num * priv->channels.params.num_tc) {
>> + for (i = 0; i < priv->channels.num * priv->channels.params.num_tc;
>> + i++) {
>> struct mlx5e_txqsq *sq = priv->txq2sq[i];
>> u8 state;
>>
>> @@ -312,15 +250,11 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>> if (err)
>> break;
>>
>> - err = mlx5e_tx_reporter_build_diagnose_output(buffers_array[buff],
>> - sq->sqn, state,
>> + err = mlx5e_tx_reporter_build_diagnose_output(msg_ctx, sq->sqn,
>> + state,
>> netif_xmit_stopped(sq->txq));
>
> This should be an array. On SQ entry : one member of array.
>
> So if you want to change it, you need to do this in 2 patches. One API,
> one the actual layout. :/
>
>
>
>> - if (err) {
>> - if (++buff == num_buffers)
>> - break;
>> - } else {
>> - i++;
>> - }
>> + if (err)
>> + break;
>> }
>>
>> mutex_unlock(&priv->state_lock);
>> @@ -330,11 +264,6 @@ static int mlx5e_tx_reporter_diagnose(struct devlink_health_reporter *reporter,
>> static const struct devlink_health_reporter_ops mlx5_tx_reporter_ops = {
>> .name = "TX",
>> .recover = mlx5e_tx_reporter_recover,
>> - .diagnose_size = MLX5E_MAX_NUM_CHANNELS * MLX5E_MAX_NUM_TC *
>> - MLX5E_TX_REPORTER_PER_SQ_MAX_LEN,
>> - .diagnose = mlx5e_tx_reporter_diagnose,
>
> So you change the callback, remove it so the dissection is broken.
This is needed in order to have this patch compiled.
>
>
>
>> - .dump_size = 0,
>> - .dump = NULL,
>
>
> This has 0 relation to the patch. Should be separate.
ack.
>
>
>> };
>>
>> #define MLX5_REPORTER_TX_GRACEFUL_PERIOD 500
>> --
>> 2.17.1
>>
^ permalink raw reply
* [PATCH net-next] mlxfw: Replace license text with SPDX identifiers and adjust copyrights
From: Shalom Toledo @ 2019-01-24 7:33 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, Ido Schimmel, Jiri Pirko, mlxsw,
Slava Shwartsman
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 35 ++----------------
.../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 35 ++----------------
.../net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c | 35 ++----------------
.../net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h | 35 ++----------------
.../ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h | 35 ++----------------
.../mellanox/mlxfw/mlxfw_mfa2_format.h | 36 ++-----------------
.../ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h | 35 ++----------------
.../mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c | 35 ++----------------
.../mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h | 36 ++-----------------
9 files changed, 20 insertions(+), 297 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
index 7a712b6b09ec..14c0c62f8e73 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#ifndef _MLXFW_H
#define _MLXFW_H
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index 2cf89126fb23..240c027e5f07 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw.c
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#define pr_fmt(fmt) "mlxfw: " fmt
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
index 993cb5ba934e..544344ac4894 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.c
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#define pr_fmt(fmt) "mlxfw_mfa2: " fmt
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h
index 20472aa139cd..5bba6ad79d34 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#ifndef _MLXFW_MFA2_H
#define _MLXFW_MFA2_H
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h
index f667942b1ea3..874c0a2474ae 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_file.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#ifndef _MLXFW_MFA2_FILE_H
#define _MLXFW_MFA2_FILE_H
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_format.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_format.h
index dd66737c033d..b001e5258091 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_format.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_format.h
@@ -1,36 +1,6 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_format.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
+
#ifndef _MLXFW_MFA2_FORMAT_H
#define _MLXFW_MFA2_FORMAT_H
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h
index cc013e77b326..33c971190bba 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#ifndef _MLXFW_MFA2_TLV_H
#define _MLXFW_MFA2_TLV_H
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c
index 0094b92a233b..017d68f1e123 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c
@@ -1,36 +1,5 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.c
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
#define pr_fmt(fmt) "MFA2: " fmt
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h
index 2c667894f3a2..633284eeded7 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h
@@ -1,36 +1,6 @@
-/*
- * drivers/net/ethernet/mellanox/mlxfw/mlxfw_mfa2_tlv_multi.h
- * Copyright (c) 2017 Mellanox Technologies. All rights reserved.
- * Copyright (c) 2017 Yotam Gigi <yotamg@mellanox.com>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the names of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * Alternatively, this software may be distributed under the terms of the
- * GNU General Public License ("GPL") version 2 as published by the Free
- * Software Foundation.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2017-2019 Mellanox Technologies. All rights reserved */
+
#ifndef _MLXFW_MFA2_TLV_MULTI_H
#define _MLXFW_MFA2_TLV_MULTI_H
--
2.17.2
^ permalink raw reply related
* Re: [PATCH net-next 1/7] devlink: Add devlink msg API
From: Eran Ben Elisha @ 2019-01-24 7:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh, Wei Yongjun
In-Reply-To: <20190123143120.GF2191@nanopsycho>
On 1/23/2019 4:31 PM, Jiri Pirko wrote:
> Tue, Jan 22, 2019 at 04:57:18PM CET, eranbe@mellanox.com wrote:
>> Devlink msg is a mechanism to pass descriptors between drivers and
>> devlink, in json-like format. The API allows the driver to add objects,
>> object pair, value array (nested attributes), value and name.
>>
>> Driver can use this API to fill the msg context in a format which can be
>> translated by the devlink to the netlink message later.
>>
>> With this API, driver does not need to declare the total size per message
>> context, and it dynamically add more messages to the context (bounded by
>> the system memory limitation only).
>> There is an implicit request to have the preliminary main objects size
>> created via this API to be upper bounded by netlink SKB size, as those
>> objects do not get fragmented between different SKBs when passed to the
>> netlink layer.
>>
>> Devlink msg will replace devlink buffers for health reporters. Devlink
>> buffers which will be deprecated and removed in the downstream patch.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> CC: Wei Yongjun <weiyongjun1@huawei.com>
>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> include/net/devlink.h | 70 ++++++
>> include/uapi/linux/devlink.h | 8 +
>> net/core/devlink.c | 455 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 533 insertions(+)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index a81a1b7a67d7..fe323e9b14e1 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -424,6 +424,7 @@ struct devlink_region;
>>
>> typedef void devlink_snapshot_data_dest_t(const void *data);
>>
>> +struct devlink_msg_ctx;
>> struct devlink_health_buffer;
>> struct devlink_health_reporter;
>>
>> @@ -615,6 +616,21 @@ int devlink_region_snapshot_create(struct devlink_region *region, u64 data_len,
>> u8 *data, u32 snapshot_id,
>> devlink_snapshot_data_dest_t *data_destructor);
>>
>> +int devlink_msg_nest_start(struct devlink_msg_ctx *msg_ctx, int attrtype);
>
>
> devlink_msg_*() functions should work with struct devlink_msg.
> devlink_msg_ctx*() functions should work with struct devlink_msg_ctx.
> Please be consistent with the naming.
>
> I think you can call this just "struct devlink_msg" of maybe "fmsg" as
> for "formatted message" - so it is not confused with any other devlink
> netlink message.
Ack.
>
>
>
>> +int devlink_msg_nest_end(struct devlink_msg_ctx *msg_ctx);
>> +int devlink_msg_put_value(struct devlink_msg_ctx *msg_ctx,
>> + void *value, u16 value_len, u8 value_nla_type);
>> +int devlink_msg_put_name(struct devlink_msg_ctx *msg_ctx,
>> + char *name);
>> +int devlink_msg_put_name_value_pair(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void *value, u16 value_len,
>> + u8 value_nla_type);
>> +int devlink_msg_put_name_value_array(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void **value, u16 *value_len,
>> + u8 *value_nla_type, int array_size);
>> +int devlink_msg_object_start(struct devlink_msg_ctx *msg_ctx, char *name);
>> +int devlink_msg_object_end(struct devlink_msg_ctx *msg_ctx, char *name);
>> +
>> int devlink_health_buffer_nest_start(struct devlink_health_buffer *buffer,
>> int attrtype);
>> void devlink_health_buffer_nest_end(struct devlink_health_buffer *buffer);
>> @@ -903,6 +919,60 @@ devlink_region_snapshot_create(struct devlink_region *region, u64 data_len,
>> return 0;
>> }
>>
>> +static inline int
>> +devlink_msg_nest_start(struct devlink_msg_ctx *msg_ctx, int attrtype)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_nest_end(struct devlink_msg_ctx *msg_ctx)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_put_value(struct devlink_msg_ctx *msg_ctx,
>> + void *value, u16 value_len, u8 value_nla_type)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_put_name(struct devlink_msg_ctx *msg_ctx,
>> + char *name)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_put_name_value_pair(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void *value, u16 value_len,
>> + u8 value_nla_type)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_put_name_value_array(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void **value, u16 *value_len,
>> + u8 *value_nla_type, int array_size)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_object_start(struct devlink_msg_ctx *msg_ctx, char *name)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int
>> +devlink_msg_object_end(struct devlink_msg_ctx *msg_ctx, char *name)
>> +{
>> + return 0;
>> +}
>> +
>> static inline int
>> devlink_health_buffer_nest_start(struct devlink_health_buffer *buffer,
>> int attrtype)
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 6b26bb2ce4dc..68eeda1d0455 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -300,6 +300,14 @@ enum devlink_attr {
>> DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE_TYPE, /* u8 */
>> DEVLINK_ATTR_HEALTH_BUFFER_OBJECT_VALUE_DATA, /* dynamic */
>>
>> + DEVLINK_ATTR_MSG_OBJECT, /* nested */
>> + DEVLINK_ATTR_MSG_OBJECT_PAIR, /* nested */
>> + DEVLINK_ATTR_MSG_OBJECT_NAME, /* string */
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE, /* nested */
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_ARRAY, /* nested */
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE, /* u8 */
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA, /* dynamic */
>> +
>> DEVLINK_ATTR_HEALTH_REPORTER, /* nested */
>> DEVLINK_ATTR_HEALTH_REPORTER_NAME, /* string */
>> DEVLINK_ATTR_HEALTH_REPORTER_STATE, /* u8 */
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 60248a53c0ad..57ca096849b3 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -4098,6 +4098,461 @@ devlink_health_buffer_snd(struct genl_info *info,
>> return err;
>> }
>>
>> +struct devlink_msg {
>
> Let's call this "struct devlink_msg_item"
Ack.
>
>
>> + struct list_head list;
>> + int attrtype;
>> + u8 nla_type;
>> + u16 len;
>> + int value[0];
>> +};
>> +
>> +struct devlink_msg_ctx {
>> + struct list_head msg_list;
>> + int max_nested_depth;
>> + int curr_nest;
>> +};
>> +
>> +#define DEVLINK_MSG_MAX_SIZE (4096 - GENL_HDRLEN)
>> +
>> +static struct devlink_msg_ctx *devlink_msg_ctx_alloc(void)
>> +{
>> + struct devlink_msg_ctx *msg_ctx;
>> +
>> + msg_ctx = kzalloc(sizeof(*msg_ctx), GFP_KERNEL);
>> + if (!msg_ctx)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + INIT_LIST_HEAD(&msg_ctx->msg_list);
>> +
>> + return msg_ctx;
>> +}
>> +
>> +static void devlink_msg_ctx_free(struct devlink_msg_ctx *msg_ctx)
>> +{
>> + struct devlink_msg *msg, *tm;
>> +
>> + list_for_each_entry_safe(msg, tm, &msg_ctx->msg_list, list) {
>> + list_del(&msg->list);
>> + kfree(msg);
>> + }
>> + kfree(msg_ctx);
>> +}
>> +
>> +int devlink_msg_nest_start(struct devlink_msg_ctx *msg_ctx, int attrtype)
>> +{
>> + struct devlink_msg *nest_msg;
>> +
>> + if (attrtype != DEVLINK_ATTR_MSG_OBJECT &&
>> + attrtype != DEVLINK_ATTR_MSG_OBJECT_PAIR &&
>> + attrtype != DEVLINK_ATTR_MSG_OBJECT_VALUE &&
>> + attrtype != DEVLINK_ATTR_MSG_OBJECT_VALUE_ARRAY)
>> + return -EINVAL;
>> +
>> + nest_msg = kzalloc(sizeof(*nest_msg), GFP_KERNEL);
>> + if (!nest_msg)
>> + return -ENOMEM;
>> +
>> + nest_msg->attrtype = attrtype;
>> + msg_ctx->curr_nest++;
>> + msg_ctx->max_nested_depth = max(msg_ctx->max_nested_depth,
>> + msg_ctx->curr_nest);
>> + list_add_tail(&nest_msg->list, &msg_ctx->msg_list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_nest_start);
>> +
>> +int devlink_msg_nest_end(struct devlink_msg_ctx *msg_ctx)
>> +{
>> + struct devlink_msg *nest_msg;
>> +
>> + WARN_ON(!msg_ctx->curr_nest);
>> + nest_msg = kzalloc(sizeof(*nest_msg), GFP_KERNEL);
>> + if (!nest_msg)
>> + return -ENOMEM;
>> +
>> + msg_ctx->curr_nest--;
>> + list_add_tail(&nest_msg->list, &msg_ctx->msg_list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_nest_end);
>> +
>> +int devlink_msg_put_value(struct devlink_msg_ctx *msg_ctx,
>> + void *value, u16 value_len, u8 value_nla_type)
>> +{
>> + struct devlink_msg *value_msg;
>> +
>> + if (value_len > DEVLINK_MSG_MAX_SIZE)
>> + return -EMSGSIZE;
>> +
>> + if (value_nla_type != NLA_U8 &&
>> + value_nla_type != NLA_U32 &&
>> + value_nla_type != NLA_U64 &&
>> + value_nla_type != NLA_NUL_STRING &&
>> + value_nla_type != NLA_BINARY)
>> + return -EINVAL;
>> +
>> + value_msg = kzalloc(sizeof(*value_msg) + value_len, GFP_KERNEL);
>
> Looks fine.
>
>
>> + if (!value_msg)
>> + return -ENOMEM;
>> +
>> + value_msg->nla_type = value_nla_type;
>> + value_msg->len = value_len;
>> + value_msg->attrtype = DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA;
>> + memcpy(&value_msg->value, value, value_len);
>> + list_add_tail(&value_msg->list, &msg_ctx->msg_list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_put_value);
>> +
>> +int devlink_msg_put_name(struct devlink_msg_ctx *msg_ctx, char *name)
>> +{
>> + struct devlink_msg *name_msg;
>> +
>> + if (strlen(name) + 1 > DEVLINK_MSG_MAX_SIZE)
>> + return -EMSGSIZE;
>> +
>> + name_msg = kzalloc(sizeof(*name_msg) + strlen(name) + 1, GFP_KERNEL);
>> + if (!name_msg)
>> + return -ENOMEM;
>> +
>> + name_msg->nla_type = NLA_NUL_STRING;
>> + name_msg->len = strlen(name) + 1;
>> + name_msg->attrtype = DEVLINK_ATTR_MSG_OBJECT_NAME;
>> + memcpy(&name_msg->value, name, name_msg->len);
>> + list_add_tail(&name_msg->list, &msg_ctx->msg_list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_put_name);
>> +
>> +int devlink_msg_put_name_value_pair(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void *value, u16 value_len,
>> + u8 value_nla_type)
>> +{
>> + int err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_PAIR);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_put_name(msg_ctx, name);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_VALUE);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_put_value(msg_ctx, value, value_len, value_nla_type);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_put_name_value_pair);
>> +
>> +int devlink_msg_put_name_value_array(struct devlink_msg_ctx *msg_ctx,
>> + char *name, void **value, u16 *value_len,
>> + u8 *value_nla_type, int array_size)
>
> This is limitting the arrays to plain values. One should be able to nest
> objects inside. If fact, that is what you can to do with sqs:
>
> object start
> name sqs
> array start
> object start
> index 0
> xxx yyy
> object end
> object start
> index 1
> xxx yyy
> object end
> array end
> object end
>
> So you need to have:
> devlink_msg_put_array_start()
> devlink_msg_put_array_end()
Sounds good, will do.
>
>
>> +{
>> + int err, i;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_PAIR);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_put_name(msg_ctx, name);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_VALUE);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx,
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_ARRAY);
>> + if (err)
>> + return err;
>> +
>> + for (i = 0; i < array_size; i++) {
>> + err = devlink_msg_nest_start(msg_ctx,
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_put_value(msg_ctx, value[i],
>> + value_len[i], value_nla_type[i]);
>> + if (err)
>> + return err;
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> + }
>> +
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_end(msg_ctx);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_put_name_value_array);
>> +
>> +int devlink_msg_object_start(struct devlink_msg_ctx *msg_ctx, char *name)
>> +{
>> + int err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT);
>> + if (err)
>> + return err;
>> +
>> + if (!name)
>> + return 0;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_PAIR);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_put_name(msg_ctx, name);
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_start(msg_ctx, DEVLINK_ATTR_MSG_OBJECT_VALUE);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_object_start);
>> +
>> +int devlink_msg_object_end(struct devlink_msg_ctx *msg_ctx, char *name)
>> +{
>> + int err;
>> +
>> + if (!name)
>> + goto end_object;
>> +
>> + err = devlink_msg_nest_end(msg_ctx); /* DEVLINK_ATTR_MSG_OBJECT_VALUE */
>> + if (err)
>> + return err;
>> +
>> + err = devlink_msg_nest_end(msg_ctx); /* DEVLINK_ATTR_MSG_OBJECT_PAIR */
>> + if (err)
>> + return err;
>> +
>> +end_object:
>> + err = devlink_msg_nest_end(msg_ctx); /* DEVLINK_ATTR_MSG_OBJECT */
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_msg_object_end);
>> +
>> +static int
>> +devlink_msg_fill_data(struct sk_buff *skb, struct devlink_msg *msg)
>> +{
>> + int err;
>> +
>> + switch (msg->nla_type) {
>> + case NLA_U8:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA,
>> + *(u8 *)msg->value);
>> + break;
>> + case NLA_U32:
>> + err = nla_put_u32(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA,
>> + *(u32 *)msg->value);
>> + break;
>> + case NLA_U64:
>> + err = nla_put_u64_64bit(skb,
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA,
>> + *(u64 *)msg->value, DEVLINK_ATTR_PAD);
>> + break;
>> + case NLA_NUL_STRING:
>> + err = nla_put_string(skb,
>> + DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA,
>> + (char *)&msg->value);
>> + break;
>> + case NLA_BINARY:
>> + err = nla_put(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA,
>> + msg->len, (void *)&msg->value);
>> + break;
>> + default:
>> + err = -EINVAL;
>> + break;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int
>> +devlink_msg_fill_type(struct sk_buff *skb, struct devlink_msg *msg)
>> +{
>> + int err;
>> +
>> + switch (msg->nla_type) {
>> + case NLA_U8:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE,
>> + NLA_U8);
>> + break;
>> + case NLA_U32:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE,
>> + NLA_U32);
>> + break;
>> + case NLA_U64:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE,
>> + NLA_U64);
>> + break;
>> + case NLA_NUL_STRING:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE,
>> + NLA_NUL_STRING);
>> + break;
>> + case NLA_BINARY:
>> + err = nla_put_u8(skb, DEVLINK_ATTR_MSG_OBJECT_VALUE_TYPE,
>> + NLA_BINARY);
>> + break;
>> + default:
>> + err = -EINVAL;
>> + break;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int
>> +devlink_msg_prepare_skb(struct sk_buff *skb, struct devlink_msg_ctx *msg_ctx,
>> + int *start)
>> +{
>> + struct nlattr **nlattr_arr;
>> + struct devlink_msg *msg;
>> + int j = 0, i = 0;
>> + int err;
>> +
>> + nlattr_arr = kcalloc(msg_ctx->max_nested_depth,
>> + sizeof(*nlattr_arr), GFP_KERNEL);
>> + if (!nlattr_arr)
>> + return -EINVAL;
>> +
>> + list_for_each_entry(msg, &msg_ctx->msg_list, list) {
>> + if (j < *start) {
>> + j++;
>> + continue;
>> + }
>> +
>> + switch (msg->attrtype) {
>> + case DEVLINK_ATTR_MSG_OBJECT:
>> + case DEVLINK_ATTR_MSG_OBJECT_PAIR:
>> + case DEVLINK_ATTR_MSG_OBJECT_VALUE:
>> + case DEVLINK_ATTR_MSG_OBJECT_VALUE_ARRAY:
>> + nlattr_arr[i] = nla_nest_start(skb, msg->attrtype);
>> + if (!nlattr_arr[i]) {
>> + err = -EMSGSIZE;
>> + goto nla_put_failure;
>> + }
>> + i++;
>> + break;
>> + case DEVLINK_ATTR_MSG_OBJECT_VALUE_DATA:
>> + err = devlink_msg_fill_data(skb, msg);
>> + if (err)
>> + goto nla_put_failure;
>> + err = devlink_msg_fill_type(skb, msg);
>> + if (err)
>> + goto nla_put_failure;
>> + break;
>> + case DEVLINK_ATTR_MSG_OBJECT_NAME:
>> + err = nla_put_string(skb, msg->attrtype,
>> + (char *)&msg->value);
>> + if (err)
>> + goto nla_put_failure;
>> + break;
>> + default: /* No attrtype */
>> + nla_nest_end(skb, nlattr_arr[--i]);
>> + break;
>> + }
>> + j++;
>> + if (!i)
>> + *start = j;
>> + }
>> +
>> + kfree(nlattr_arr);
>> + return 0;
>> +
>> +nla_put_failure:
>> + for (j = 0; j < i; j++)
>> + nla_nest_cancel(skb, nlattr_arr[j]);
>> + kfree(nlattr_arr);
>> + return err;
>> +}
>> +
>> +static int devlink_msg_snd(struct genl_info *info,
>> + enum devlink_command cmd, int flags,
>> + struct devlink_msg_ctx *msg_ctx)
>> +{
>> + struct sk_buff *skb;
>> + struct nlmsghdr *nlh;
>> + int err, index = 0;
>> + bool last = false;
>> + void *hdr;
>> +
>> + while (!last) {
>> + int tmp_index = index;
>> +
>> + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
>> + &devlink_nl_family, NLM_F_MULTI, cmd);
>> + if (!hdr)
>> + goto nla_put_failure;
>> +
>> + err = devlink_msg_prepare_skb(skb, msg_ctx, &index);
>> + if (!err)
>> + last = true;
>> + else if (err != -EMSGSIZE || tmp_index == index)
>> + goto nla_put_failure;
>> +
>> + genlmsg_end(skb, hdr);
>> + err = genlmsg_reply(skb, info);
>> + if (err)
>> + return err;
>
>
> Looks fine.
>
>
>> + }
>> +
>> + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> + if (!skb)
>> + return -ENOMEM;
>> + nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq,
>> + NLMSG_DONE, 0, flags | NLM_F_MULTI);
>> + err = genlmsg_reply(skb, info);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +
>> +nla_put_failure:
>> + err = -EIO;
>> + nlmsg_free(skb);
>> + return err;
>> +}
>> +
>> struct devlink_health_reporter {
>> struct list_head list;
>> struct devlink_health_buffer **dump_buffers_array;
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH 4/4] can: flexcan: fix NULL pointer exception during bringup
From: Uwe Kleine-König @ 2019-01-24 7:26 UTC (permalink / raw)
To: Sasha Levin; +Cc: Marc Kleine-Budde, netdev, davem, linux-can, stable
In-Reply-To: <20190123225745.3D239218A2@mail.kernel.org>
Hello Sasha,
On Wed, Jan 23, 2019 at 10:57:44PM +0000, Sasha Levin wrote:
> [This is an automated email]
Not sure if I only state the obvious that was just missed by your
automation.
>
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: cbffaf7aa09e can: flexcan: Always use last mailbox for TX.
>
> The bot has tested the following trees: v4.20.3, v4.19.16.
>
> v4.20.3: Failed to apply! Possible dependencies:
> 0517961ccdf1 ("can: flexcan: Add provision for variable payload size")
> 22233f7bf2c9 ("can: flexcan: FLEXCAN_IFLAG_MB: add () around macro argument")
> 5156c7b11f35 ("can: flexcan: move rx_offload_add() from flexcan_probe() to flexcan_open()")
> 7ad0f53a394b ("can: flexcan: flexcan_chip_start(): enable loopback mode in flexcan")
> de3578c198c6 ("can: flexcan: add self wakeup support")
>
> v4.19.16: Failed to apply! Possible dependencies:
> 0517961ccdf1 ("can: flexcan: Add provision for variable payload size")
> 22233f7bf2c9 ("can: flexcan: FLEXCAN_IFLAG_MB: add () around macro argument")
> 5156c7b11f35 ("can: flexcan: move rx_offload_add() from flexcan_probe() to flexcan_open()")
> 7ad0f53a394b ("can: flexcan: flexcan_chip_start(): enable loopback mode in flexcan")
> de3578c198c6 ("can: flexcan: add self wakeup support")
>
>
> How should we proceed with this patch?
When I posted the commit I also posted one that fits on top of v4.19.x.
I would expect that the same also fits on v4.20.x. If not, tell me.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH net-next v3 0/4] net: phy: improve starting PHY
From: S-k, Shyam-sundar @ 2019-01-24 7:14 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, Florian Fainelli, David Miller
Cc: netdev@vger.kernel.org
In-Reply-To: <bddac7b8-a4be-196e-f75e-32a2f44d8807@gmail.com>
On 1/23/2019 11:55 AM, Heiner Kallweit wrote:
> This patch series improves few aspects of starting the PHY.
>
> v2:
> - improve a warning in patch 4
> v3:
> - extend commit message for patch 2
>
> Heiner Kallweit (4):
> net: phy: start state machine in phy_start only
> net: phy: warn if phy_start is called from invalid state
> net: phy: start interrupts in phy_start
> net: phy: change phy_start_interrupts to phy_request_interrupt
>
> drivers/net/phy/phy.c | 64 ++++++++++++++++++------------------
> drivers/net/phy/phy_device.c | 5 ++-
> drivers/net/phy/phylink.c | 5 ++-
> include/linux/phy.h | 2 +-
> 4 files changed, 37 insertions(+), 39 deletions(-)
Changes works well on AMD xgbe hardware.
Tested-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
^ permalink raw reply
* [PATCH] net: usb: asix: ax88772_bind return error when hw_reset fail
From: Zhang Run @ 2019-01-24 5:48 UTC (permalink / raw)
To: davem
Cc: marcel.ziswiler, lynxis, linux-usb, netdev, linux-kernel,
xue.zhihong, wang.yi59, Zhang Run
The ax88772_bind() should return error code immediately when the PHY
was not reset properly through ax88772a_hw_reset().
Otherwise, The asix_get_phyid() will block when get the PHY
Identifier from the PHYSID1 MII registers through asix_mdio_read()
due to the PHY isn't ready. Furthermore, it will produce a lot of
error message cause system crash.As follows:
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
reg index 0x0000: -71
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to send
software reset: ffffffb9
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
reg index 0x0000: -71
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to enable
software MII access
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to read
reg index 0x0000: -71
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
reg index 0x0000: -71
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to enable
software MII access
asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to read
reg index 0x0000: -71
...
Signed-off-by: Zhang Run <zhang.run@zte.com.cn>
Reviewed-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/usb/asix_devices.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index b654f05..3d93993 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -739,8 +739,13 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &chipcode, 0);
chipcode &= AX_CHIPCODE_MASK;
- (chipcode == AX_AX88772_CHIPCODE) ? ax88772_hw_reset(dev, 0) :
- ax88772a_hw_reset(dev, 0);
+ ret = (chipcode == AX_AX88772_CHIPCODE) ? ax88772_hw_reset(dev, 0) :
+ ax88772a_hw_reset(dev, 0);
+
+ if (ret < 0) {
+ netdev_dbg(dev->net, "Failed to reset AX88772: %d\n", ret);
+ return ret;
+ }
/* Read PHYID register *AFTER* the PHY was reset properly */
phyid = asix_get_phyid(dev);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 bpf-next] bpf: use the proper optlen when doing strncpy in bpf_getsockopt()
From: Yafang Shao @ 2019-01-24 5:16 UTC (permalink / raw)
To: kafai, brakmo, ast, daniel; +Cc: netdev, shaoyafang, Yafang Shao
As strncpy(..., TCP_CA_NAME_MAX-1) is used in bpf_setsockopt(),
this change will make it more consistent with the bpf_setsockopt() above.
Acked-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Lawrence Brakmo <brakmo@fb.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
net/core/filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 447dd1b..7a4de22 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4262,7 +4262,7 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
if (!icsk->icsk_ca_ops || optlen <= 1)
goto err_clear;
- strncpy(optval, icsk->icsk_ca_ops->name, optlen);
+ strncpy(optval, icsk->icsk_ca_ops->name, optlen - 1);
optval[optlen - 1] = 0;
break;
case TCP_SAVED_SYN:
--
1.8.3.1
^ permalink raw reply related
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