Netdev List
 help / color / mirror / Atom feed
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Marcel Holtmann @ 2019-08-30 12:43 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: megous, Chen-Yu Tsai, Rob Herring, Johan Hedberg, Mark Rutland,
	David S. Miller, netdev, devicetree, linux-kernel,
	linux-arm-kernel, linux-bluetooth
In-Reply-To: <20190830092104.odipmbflounqpffo@flea>

Hi Maxime,

>>> (Resend to add missing lists, sorry for the noise.)
>>> 
>>> This series implements bluetooth support for Xunlong Orange Pi 3 board.
>>> 
>>> The board uses AP6256 WiFi/BT 5.0 chip.
>>> 
>>> Summary of changes:
>>> 
>>> - add more delay to let initialize the chip
>>> - let the kernel detect firmware file path
>>> - add new compatible and update dt-bindings
>>> - update Orange Pi 3 / H6 DTS
>>> 
>>> Please take a look.
>>> 
>>> thank you and regards,
>>> Ondrej Jirman
>>> 
>>> Ondrej Jirman (5):
>>> dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
>>> bluetooth: bcm: Add support for loading firmware for BCM4345C5
>>> bluetooth: hci_bcm: Give more time to come out of reset
>>> arm64: dts: allwinner: h6: Add pin configs for uart1
>>> arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
>>> 
>>> .../bindings/net/broadcom-bluetooth.txt       |  1 +
>>> .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 19 +++++++++++++++++++
>>> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 10 ++++++++++
>>> drivers/bluetooth/btbcm.c                     |  3 +++
>>> drivers/bluetooth/hci_bcm.c                   |  3 ++-
>>> 5 files changed, 35 insertions(+), 1 deletion(-)
>> 
>> all 5 patches have been applied to bluetooth-next tree.
> 
> The DTS patches (last 2) should go through the arm-soc tree, can you
> drop them?

why is that? We have included DTS changes for Bluetooth devices directly all the time. What is different with this hardware?

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
From: Cornelia Huck @ 2019-08-30 12:40 UTC (permalink / raw)
  To: Parav Pandit
  Cc: alex.williamson, jiri, kwankhede, davem, kvm, linux-kernel,
	netdev
In-Reply-To: <20190829111904.16042-3-parav@mellanox.com>

On Thu, 29 Aug 2019 06:19:00 -0500
Parav Pandit <parav@mellanox.com> wrote:

> Mdev alias should be unique among all the mdevs, so that when such alias
> is used by the mdev users to derive other objects, there is no
> collision in a given system.
> 
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> 
> ---
> Changelog:
> v1->v2:
>  - Moved alias NULL check at beginning
> v0->v1:
>  - Fixed inclusiong of alias for NULL check
>  - Added ratelimited debug print for sha1 hash collision error
> ---
>  drivers/vfio/mdev/mdev_core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index 3bdff0469607..c9bf2ac362b9 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -388,6 +388,13 @@ int mdev_device_create(struct kobject *kobj, struct device *dev,
>  			ret = -EEXIST;
>  			goto mdev_fail;
>  		}
> +		if (alias && tmp->alias && strcmp(alias, tmp->alias) == 0) {
> +			mutex_unlock(&mdev_list_lock);
> +			ret = -EEXIST;
> +			dev_dbg_ratelimited(dev, "Hash collision in alias creation for UUID %pUl\n",
> +					    uuid);
> +			goto mdev_fail;
> +		}
>  	}
>  
>  	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);

Any reason not to merge this into the first patch?

^ permalink raw reply

* Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-30 12:39 UTC (permalink / raw)
  To: Parav Pandit
  Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
	davem@davemloft.net, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB48660877881F7A2D757A9C82D1BD0@AM0PR05MB4866.eurprd05.prod.outlook.com>

On Fri, 30 Aug 2019 12:33:22 +0000
Parav Pandit <parav@mellanox.com> wrote:

> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Friday, August 30, 2019 2:47 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> > 
> > On Thu, 29 Aug 2019 06:18:59 -0500
> > Parav Pandit <parav@mellanox.com> wrote:
> >   
> > > Some vendor drivers want an identifier for an mdev device that is
> > > shorter than the UUID, due to length restrictions in the consumers of
> > > that identifier.
> > >
> > > Add a callback that allows a vendor driver to request an alias of a
> > > specified length to be generated for an mdev device. If generated,
> > > that alias is checked for collisions.
> > >
> > > It is an optional attribute.
> > > mdev alias is generated using sha1 from the mdev name.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > >
> > > ---
> > > Changelog:
> > > v1->v2:
> > >  - Kept mdev_device naturally aligned
> > >  - Added error checking for crypt_*() calls
> > >  - Corrected a typo from 'and' to 'an'
> > >  - Changed return type of generate_alias() from int to char*
> > > v0->v1:
> > >  - Moved alias length check outside of the parent lock
> > >  - Moved alias and digest allocation from kvzalloc to kzalloc
> > >  - &alias[0] changed to alias
> > >  - alias_length check is nested under get_alias_length callback check
> > >  - Changed comments to start with an empty line
> > >  - Fixed cleaunup of hash if mdev_bus_register() fails
> > >  - Added comment where alias memory ownership is handed over to mdev
> > > device
> > >  - Updated commit log to indicate motivation for this feature
> > > ---
> > >  drivers/vfio/mdev/mdev_core.c    | 123  
> > ++++++++++++++++++++++++++++++-  
> > >  drivers/vfio/mdev/mdev_private.h |   5 +-
> > >  drivers/vfio/mdev/mdev_sysfs.c   |  13 ++--
> > >  include/linux/mdev.h             |   4 +
> > >  4 files changed, 135 insertions(+), 10 deletions(-)

> > ...and detached from the local variable here. Who is freeing it? The comment
> > states that it is done by the mdev, but I don't see it?
> >   
> mdev_device_free() frees it.

Ah yes, I overlooked the kfree().

> once its assigned to mdev, mdev is the owner of it.
> 
> > This detour via the local variable looks weird to me. Can you either create the
> > alias directly in the mdev (would need to happen later in the function, but I'm
> > not sure why you generate the alias before checking for duplicates anyway), or
> > do an explicit copy?  
> Alias duplicate check is done after generating it, because duplicate alias are not allowed.
> The probability of collision is rare.
> So it is speculatively generated without hold the lock, because there is no need to hold the lock.
> It is compared along with guid while mutex lock is held in single loop.
> And if it is duplicate, there is no need to allocate mdev.
> 
> It will be sub optimal to run through the mdev list 2nd time after mdev creation and after generating alias for duplicate check.

Ok, but what about copying it? I find this "set local variable to NULL
after ownership is transferred" pattern a bit unintuitive. Copying it
to the mdev (and then unconditionally freeing it) looks more obvious to
me.

^ permalink raw reply

* RE: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-30 12:33 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
	davem@davemloft.net, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830111720.04aa54e9.cohuck@redhat.com>



> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 2:47 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> 
> On Thu, 29 Aug 2019 06:18:59 -0500
> Parav Pandit <parav@mellanox.com> wrote:
> 
> > Some vendor drivers want an identifier for an mdev device that is
> > shorter than the UUID, due to length restrictions in the consumers of
> > that identifier.
> >
> > Add a callback that allows a vendor driver to request an alias of a
> > specified length to be generated for an mdev device. If generated,
> > that alias is checked for collisions.
> >
> > It is an optional attribute.
> > mdev alias is generated using sha1 from the mdev name.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> >
> > ---
> > Changelog:
> > v1->v2:
> >  - Kept mdev_device naturally aligned
> >  - Added error checking for crypt_*() calls
> >  - Corrected a typo from 'and' to 'an'
> >  - Changed return type of generate_alias() from int to char*
> > v0->v1:
> >  - Moved alias length check outside of the parent lock
> >  - Moved alias and digest allocation from kvzalloc to kzalloc
> >  - &alias[0] changed to alias
> >  - alias_length check is nested under get_alias_length callback check
> >  - Changed comments to start with an empty line
> >  - Fixed cleaunup of hash if mdev_bus_register() fails
> >  - Added comment where alias memory ownership is handed over to mdev
> > device
> >  - Updated commit log to indicate motivation for this feature
> > ---
> >  drivers/vfio/mdev/mdev_core.c    | 123
> ++++++++++++++++++++++++++++++-
> >  drivers/vfio/mdev/mdev_private.h |   5 +-
> >  drivers/vfio/mdev/mdev_sysfs.c   |  13 ++--
> >  include/linux/mdev.h             |   4 +
> >  4 files changed, 135 insertions(+), 10 deletions(-)
> >
> 
> (...)
> 
> > +static const char *
> > +generate_alias(const char *uuid, unsigned int max_alias_len) {
> > +	struct shash_desc *hash_desc;
> > +	unsigned int digest_size;
> > +	unsigned char *digest;
> > +	unsigned int alias_len;
> > +	char *alias;
> > +	int ret;
> > +
> > +	/*
> > +	 * Align to multiple of 2 as bin2hex will generate
> > +	 * even number of bytes.
> > +	 */
> > +	alias_len = roundup(max_alias_len, 2);
> > +	alias = kzalloc(alias_len + 1, GFP_KERNEL);
> 
> This function allocates alias...
> 
> > +	if (!alias)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	/* Allocate and init descriptor */
> > +	hash_desc = kvzalloc(sizeof(*hash_desc) +
> > +			     crypto_shash_descsize(alias_hash),
> > +			     GFP_KERNEL);
> > +	if (!hash_desc) {
> > +		ret = -ENOMEM;
> > +		goto desc_err;
> > +	}
> > +
> > +	hash_desc->tfm = alias_hash;
> > +
> > +	digest_size = crypto_shash_digestsize(alias_hash);
> > +
> > +	digest = kzalloc(digest_size, GFP_KERNEL);
> > +	if (!digest) {
> > +		ret = -ENOMEM;
> > +		goto digest_err;
> > +	}
> > +	ret = crypto_shash_init(hash_desc);
> > +	if (ret)
> > +		goto hash_err;
> > +
> > +	ret = crypto_shash_update(hash_desc, uuid, UUID_STRING_LEN);
> > +	if (ret)
> > +		goto hash_err;
> > +
> > +	ret = crypto_shash_final(hash_desc, digest);
> > +	if (ret)
> > +		goto hash_err;
> > +
> > +	bin2hex(alias, digest, min_t(unsigned int, digest_size, alias_len / 2));
> > +	/*
> > +	 * When alias length is odd, zero out an additional last byte
> > +	 * that bin2hex has copied.
> > +	 */
> > +	if (max_alias_len % 2)
> > +		alias[max_alias_len] = 0;
> > +
> > +	kfree(digest);
> > +	kvfree(hash_desc);
> > +	return alias;
> 
> ...and returns it here on success...
> 
> > +
> > +hash_err:
> > +	kfree(digest);
> > +digest_err:
> > +	kvfree(hash_desc);
> > +desc_err:
> > +	kfree(alias);
> > +	return ERR_PTR(ret);
> > +}
> > +
> > +int mdev_device_create(struct kobject *kobj, struct device *dev,
> > +		       const char *uuid_str, const guid_t *uuid)
> >  {
> >  	int ret;
> >  	struct mdev_device *mdev, *tmp;
> >  	struct mdev_parent *parent;
> >  	struct mdev_type *type = to_mdev_type(kobj);
> > +	const char *alias = NULL;
> >
> >  	parent = mdev_get_parent(type->parent);
> >  	if (!parent)
> >  		return -EINVAL;
> >
> > +	if (parent->ops->get_alias_length) {
> > +		unsigned int alias_len;
> > +
> > +		alias_len = parent->ops->get_alias_length();
> > +		if (alias_len) {
> > +			alias = generate_alias(uuid_str, alias_len);
> 
> ...to be saved into a local variable here...
> 
> > +			if (IS_ERR(alias)) {
> > +				ret = PTR_ERR(alias);
> > +				goto alias_fail;
> > +			}
> > +		}
> > +	}
> >  	mutex_lock(&mdev_list_lock);
> >
> >  	/* Check for duplicate */
> > @@ -300,6 +398,12 @@ int mdev_device_create(struct kobject *kobj,
> >  	}
> >
> >  	guid_copy(&mdev->uuid, uuid);
> > +	mdev->alias = alias;
> 
> ...and reassigned to the mdev member here...
> 
> > +	/*
> > +	 * At this point alias memory is owned by the mdev.
> > +	 * Mark it NULL, so that only mdev can free it.
> > +	 */
> > +	alias = NULL;
> 
> ...and detached from the local variable here. Who is freeing it? The comment
> states that it is done by the mdev, but I don't see it?
> 
mdev_device_free() frees it.
once its assigned to mdev, mdev is the owner of it.

> This detour via the local variable looks weird to me. Can you either create the
> alias directly in the mdev (would need to happen later in the function, but I'm
> not sure why you generate the alias before checking for duplicates anyway), or
> do an explicit copy?
Alias duplicate check is done after generating it, because duplicate alias are not allowed.
The probability of collision is rare.
So it is speculatively generated without hold the lock, because there is no need to hold the lock.
It is compared along with guid while mutex lock is held in single loop.
And if it is duplicate, there is no need to allocate mdev.

It will be sub optimal to run through the mdev list 2nd time after mdev creation and after generating alias for duplicate check.

> 
> >  	list_add(&mdev->next, &mdev_list);
> >  	mutex_unlock(&mdev_list_lock);
> >
> > @@ -346,6 +450,8 @@ int mdev_device_create(struct kobject *kobj,
> >  	up_read(&parent->unreg_sem);
> >  	put_device(&mdev->dev);
> >  mdev_fail:
> > +	kfree(alias);
> > +alias_fail:
> >  	mdev_put_parent(parent);
> >  	return ret;
> >  }
> 
> (...)

^ permalink raw reply

* [PATCH net 5/5] net: aquantia: fix out of memory condition on rx side
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

On embedded environments with hard memory limits it is a normal although
rare case when skb can't be allocated on rx part under high traffic.

In such OOM cases napi_complete_done() was not called.
So the napi object became in an invalid state like it is "scheduled".
Kernel do not re-schedules the poll of that napi object.

Consequently, kernel can not remove that object the system hangs on
`ifconfig down` waiting for a poll.

We are fixing this by gracefully closing napi poll routine with correct
invocation of napi_complete_done.

This was reproduced with artificially failing the allocation of skb to
simulate an "out of memory" error case and check that traffic does
not get stuck.

Fixes: 970a2e9864b0 ("net: ethernet: aquantia: Vector operations")
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_vec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
index 715685aa48c3..28892b8acd0e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
@@ -86,6 +86,7 @@ static int aq_vec_poll(struct napi_struct *napi, int budget)
 			}
 		}
 
+err_exit:
 		if (!was_tx_cleaned)
 			work_done = budget;
 
@@ -95,7 +96,7 @@ static int aq_vec_poll(struct napi_struct *napi, int budget)
 					1U << self->aq_ring_param.vec_idx);
 		}
 	}
-err_exit:
+
 	return work_done;
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net 4/5] net: aquantia: linkstate irq should be oneshot
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

Declaring threaded irq handler should also indicate the irq is
oneshot. It is oneshot indeed, because HW implements irq automasking
on trigger.

Not declaring this causes some kernel configurations to fail
on interface up, because request_threaded_irq returned an err code.

The issue was originally hidden on normal x86_64 configuration with
latest kernel, because depending on interrupt controller, irq driver
added ONESHOT flag on its own.

Issue was observed on older kernels (4.14) where no such logic exists.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
Reported-by: Michael Symolkin <Michael.Symolkin@aquantia.com>
Fixes: 4c83f170b3ac ("net: aquantia: link status irq handling")
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index e1392766e21e..8f66e7817811 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -393,7 +393,7 @@ int aq_nic_start(struct aq_nic_s *self)
 						   self->aq_nic_cfg.link_irq_vec);
 			err = request_threaded_irq(irqvec, NULL,
 						   aq_linkstate_threaded_isr,
-						   IRQF_SHARED,
+						   IRQF_SHARED | IRQF_ONESHOT,
 						   self->ndev->name, self);
 			if (err < 0)
 				goto err_exit;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net 3/5] net: aquantia: reapply vlan filters on up
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

In case of device reconfiguration the driver may reset the device invisible
for other modules, vlan module in particular. So vlans will not be
removed&created and vlan filters will not be configured in the device.
The patch reapplies the vlan filters at device start.

Fixes: 7975d2aff5afb ("net: aquantia: add support of rx-vlan-filter offload")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_main.c b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
index 100722ad5c2d..b4a0fb281e69 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_main.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
@@ -61,6 +61,10 @@ static int aq_ndev_open(struct net_device *ndev)
 	if (err < 0)
 		goto err_exit;
 
+	err = aq_filters_vlans_update(aq_nic);
+	if (err < 0)
+		goto err_exit;
+
 	err = aq_nic_start(aq_nic);
 	if (err < 0)
 		goto err_exit;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net 2/5] net: aquantia: fix limit of vlan filters
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

Fix a limit condition of vlans on the interface before setting vlan
promiscuous mode

Fixes: 48dd73d08d4dd ("net: aquantia: fix vlans not working over bridged network")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
index b13704544a23..aee827f07c16 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
@@ -844,7 +844,7 @@ int aq_filters_vlans_update(struct aq_nic_s *aq_nic)
 		return err;
 
 	if (aq_nic->ndev->features & NETIF_F_HW_VLAN_CTAG_FILTER) {
-		if (hweight < AQ_VLAN_MAX_FILTERS && hweight > 0) {
+		if (hweight <= AQ_VLAN_MAX_FILTERS && hweight > 0) {
 			err = aq_hw_ops->hw_filter_vlan_ctrl(aq_hw,
 				!(aq_nic->packet_filter & IFF_PROMISC));
 			aq_nic->aq_nic_cfg.is_vlan_force_promisc = false;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net 1/5] net: aquantia: fix removal of vlan 0
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Dmitry Bogdanov
In-Reply-To: <cover.1567163402.git.igor.russkikh@aquantia.com>

From: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>

Due to absence of checking against the rx flow rule when vlan 0 is being
removed, the other rule could be removed instead of the rule with vlan 0

Fixes: 7975d2aff5afb ("net: aquantia: add support of rx-vlan-filter offload")
Signed-off-by: Dmitry Bogdanov <dmitry.bogdanov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
index 440690b18734..b13704544a23 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_filters.c
@@ -431,7 +431,8 @@ int aq_del_fvlan_by_vlan(struct aq_nic_s *aq_nic, u16 vlan_id)
 		if (be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id)
 			break;
 	}
-	if (rule && be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id) {
+	if (rule && rule->type == aq_rx_filter_vlan &&
+	    be16_to_cpu(rule->aq_fsp.h_ext.vlan_tci) == vlan_id) {
 		struct ethtool_rxnfc cmd;
 
 		cmd.fs.location = rule->aq_fsp.location;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net 0/5] net: aquantia: fixes on vlan filters and other conditions
From: Igor Russkikh @ 2019-08-30 12:08 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh

Here is a set of various bug fixes related to vlan filter offload and
two other rare cases.

Dmitry Bogdanov (4):
  net: aquantia: fix removal of vlan 0
  net: aquantia: fix limit of vlan filters
  net: aquantia: reapply vlan filters on up
  net: aquantia: fix out of memory condition on rx side

Igor Russkikh (1):
  net: aquantia: linkstate irq should be oneshot

 drivers/net/ethernet/aquantia/atlantic/aq_filters.c | 5 +++--
 drivers/net/ethernet/aquantia/atlantic/aq_main.c    | 4 ++++
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c     | 2 +-
 drivers/net/ethernet/aquantia/atlantic/aq_vec.c     | 3 ++-
 4 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH 1/5] netfilter: xt_physdev: Fix spurious error message in physdev_mt_check
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>

From: Todd Seidelmann <tseidelmann@linode.com>

Simplify the check in physdev_mt_check() to emit an error message
only when passed an invalid chain (ie, NF_INET_LOCAL_OUT).
This avoids cluttering up the log with errors against valid rules.

For large/heavily modified rulesets, current behavior can quickly
overwhelm the ring buffer, because this function gets called on
every change, regardless of the rule that was changed.

Signed-off-by: Todd Seidelmann <tseidelmann@linode.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_physdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c
index ead7c6022208..b92b22ce8abd 100644
--- a/net/netfilter/xt_physdev.c
+++ b/net/netfilter/xt_physdev.c
@@ -101,11 +101,9 @@ static int physdev_mt_check(const struct xt_mtchk_param *par)
 	if (info->bitmask & (XT_PHYSDEV_OP_OUT | XT_PHYSDEV_OP_ISOUT) &&
 	    (!(info->bitmask & XT_PHYSDEV_OP_BRIDGED) ||
 	     info->invert & XT_PHYSDEV_OP_BRIDGED) &&
-	    par->hook_mask & ((1 << NF_INET_LOCAL_OUT) |
-	    (1 << NF_INET_FORWARD) | (1 << NF_INET_POST_ROUTING))) {
+	    par->hook_mask & (1 << NF_INET_LOCAL_OUT)) {
 		pr_info_ratelimited("--physdev-out and --physdev-is-out only supported in the FORWARD and POSTROUTING chains with bridged traffic\n");
-		if (par->hook_mask & (1 << NF_INET_LOCAL_OUT))
-			return -EINVAL;
+		return -EINVAL;
 	}
 
 	if (!brnf_probed) {
-- 
2.11.0


^ permalink raw reply related

* [PATCH 3/5] netfilter: conntrack: make sysctls per-namespace again
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

When I merged the extension sysctl tables with the main one I forgot to
reset them on netns creation.  They currently read/write init_net settings.

Fixes: d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
Fixes: cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
Reported-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_standalone.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index e0d392cb3075..0006503d2da9 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -1037,9 +1037,14 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
 	table[NF_SYSCTL_CT_COUNT].data = &net->ct.count;
 	table[NF_SYSCTL_CT_CHECKSUM].data = &net->ct.sysctl_checksum;
 	table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid;
+	table[NF_SYSCTL_CT_ACCT].data = &net->ct.sysctl_acct;
+	table[NF_SYSCTL_CT_HELPER].data = &net->ct.sysctl_auto_assign_helper;
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 	table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events;
 #endif
+#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
+	table[NF_SYSCTL_CT_TIMESTAMP].data = &net->ct.sysctl_tstamp;
+#endif
 	table[NF_SYSCTL_CT_PROTO_TIMEOUT_GENERIC].data = &nf_generic_pernet(net)->timeout;
 	table[NF_SYSCTL_CT_PROTO_TIMEOUT_ICMP].data = &nf_icmp_pernet(net)->timeout;
 	table[NF_SYSCTL_CT_PROTO_TIMEOUT_ICMPV6].data = &nf_icmpv6_pernet(net)->timeout;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 4/5] netfilter: nf_flow_table: clear skb tstamp before xmit
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

If 'fq' qdisc is used and a program has requested timestamps,
skb->tstamp needs to be cleared, else fq will treat these as
'transmit time'.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_ip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index d68c801dd614..b9e7dd6e60ce 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -228,7 +228,6 @@ static unsigned int nf_flow_xmit_xfrm(struct sk_buff *skb,
 {
 	skb_orphan(skb);
 	skb_dst_set_noref(skb, dst);
-	skb->tstamp = 0;
 	dst_output(state->net, state->sk, skb);
 	return NF_STOLEN;
 }
@@ -284,6 +283,7 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
 	flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
 	iph = ip_hdr(skb);
 	ip_decrease_ttl(iph);
+	skb->tstamp = 0;
 
 	if (unlikely(dst_xfrm(&rt->dst))) {
 		memset(skb->cb, 0, sizeof(struct inet_skb_parm));
@@ -512,6 +512,7 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
 	flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
 	ip6h = ipv6_hdr(skb);
 	ip6h->hop_limit--;
+	skb->tstamp = 0;
 
 	if (unlikely(dst_xfrm(&rt->dst))) {
 		memset(skb->cb, 0, sizeof(struct inet6_skb_parm));
-- 
2.11.0


^ permalink raw reply related

* [PATCH 5/5] netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>

From: wenxu <wenxu@ucloud.cn>

Get the vlan_proto of ingress bridge in network byteorder as userspace
expects. Otherwise this is inconsistent with NFT_META_PROTOCOL.

Fixes: 2a3a93ef0ba5 ("netfilter: nft_meta_bridge: Add NFT_META_BRI_IIFVPROTO support")
Signed-off-by: wenxu <wenxu@ucloud.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/netfilter/nft_meta_bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index 1804e867f715..7c9e92b2f806 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -53,7 +53,7 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
 			goto err;
 
 		br_vlan_get_proto(br_dev, &p_proto);
-		nft_reg_store16(dest, p_proto);
+		nft_reg_store16(dest, htons(p_proto));
 		return;
 	}
 	default:
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/5] netfilter: nf_conntrack_ftp: Fix debug output
From: Pablo Neira Ayuso @ 2019-08-30 12:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20190830120704.6147-1-pablo@netfilter.org>

From: Thomas Jarosch <thomas.jarosch@intra2net.com>

The find_pattern() debug output was printing the 'skip' character.
This can be a NULL-byte and messes up further pr_debug() output.

Output without the fix:
kernel: nf_conntrack_ftp: Pattern matches!
kernel: nf_conntrack_ftp: Skipped up to `<7>nf_conntrack_ftp: find_pattern `PORT': dlen = 8
kernel: nf_conntrack_ftp: find_pattern `EPRT': dlen = 8

Output with the fix:
kernel: nf_conntrack_ftp: Pattern matches!
kernel: nf_conntrack_ftp: Skipped up to 0x0 delimiter!
kernel: nf_conntrack_ftp: Match succeeded!
kernel: nf_conntrack_ftp: conntrack_ftp: match `172,17,0,100,200,207' (20 bytes at 4150681645)
kernel: nf_conntrack_ftp: find_pattern `PORT': dlen = 8

Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_ftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c
index 0ecb3e289ef2..8d96738b7dfd 100644
--- a/net/netfilter/nf_conntrack_ftp.c
+++ b/net/netfilter/nf_conntrack_ftp.c
@@ -322,7 +322,7 @@ static int find_pattern(const char *data, size_t dlen,
 		i++;
 	}
 
-	pr_debug("Skipped up to `%c'!\n", skip);
+	pr_debug("Skipped up to 0x%hhx delimiter!\n", skip);
 
 	*numoff = i;
 	*numlen = getnum(data + i, dlen - i, cmd, term, numoff);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 0/5] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2019-08-30 12:06 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi,

The following patchset contains Netfilter fixes for net:

1) Spurious warning when loading rules using the physdev match,
   from Todd Seidelmann.

2) Fix FTP conntrack helper debugging output, from Thomas Jarosch.

3) Restore per-netns nf_conntrack_{acct,helper,timeout} sysctl knobs,
   from Florian Westphal.

4) Clear skbuff timestamp from the flowtable datapath, also from Florian.

5) Fix incorrect byteorder of NFT_META_BRI_IIFVPROTO, from wenxu.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks.

----------------------------------------------------------------

The following changes since commit f53a7ad189594a112167efaf17ea8d0242b5ac00:

  r8152: Set memory to all 0xFFs on failed reg reads (2019-08-25 19:52:59 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD

for you to fetch changes up to daf1de9078792a4d60e36aa7ecf3aadca65277c2:

  netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder (2019-08-30 02:49:04 +0200)

----------------------------------------------------------------
Florian Westphal (2):
      netfilter: conntrack: make sysctls per-namespace again
      netfilter: nf_flow_table: clear skb tstamp before xmit

Thomas Jarosch (1):
      netfilter: nf_conntrack_ftp: Fix debug output

Todd Seidelmann (1):
      netfilter: xt_physdev: Fix spurious error message in physdev_mt_check

wenxu (1):
      netfilter: nft_meta_bridge: Fix get NFT_META_BRI_IIFVPROTO in network byteorder

 net/bridge/netfilter/nft_meta_bridge.c  | 2 +-
 net/netfilter/nf_conntrack_ftp.c        | 2 +-
 net/netfilter/nf_conntrack_standalone.c | 5 +++++
 net/netfilter/nf_flow_table_ip.c        | 3 ++-
 net/netfilter/xt_physdev.c              | 6 ++----
 5 files changed, 11 insertions(+), 7 deletions(-)

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/4] tools: bpftool: improve bpftool build experience
From: Ilya Leoshkevich @ 2019-08-30 11:42 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev, oss-drivers,
	Lorenz Bauer, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>

> Am 30.08.2019 um 13:00 schrieb Quentin Monnet <quentin.monnet@netronome.com>:
> 
> Hi,
> This set attempts to make it easier to build bpftool, in particular when
> passing a specific output directory. This is a follow-up to the
> conversation held last month by Lorenz, Ilya and Jakub [0].
> 
> The first patch is a minor fix to bpftool's Makefile, regarding the
> retrieval of kernel version (which currently prints a non-relevant make
> warning on some invocations).
> 
> Second patch improves the Makefile commands to support more "make"
> invocations, or to fix building with custom output directory. On Jakub's
> suggestion, a script is also added to BPF selftests in order to keep track
> of the supported build variants.
> 
> Building bpftool with "make tools/bpf" from the top of the repository
> generates files in "libbpf/" and "feature/" directories under tools/bpf/
> and tools/bpf/bpftool/. The third patch ensures such directories are taken
> care of on "make clean", and add them to the relevant .gitignore files.
> 
> At last, fourth patch is a sligthly modified version of Ilya's fix
> regarding libbpf.a appearing twice on the linking command for bpftool.
> 
> [0] https://lore.kernel.org/bpf/CACAyw9-CWRHVH3TJ=Tke2x8YiLsH47sLCijdp=V+5M836R9aAA@mail.gmail.com/
> 
> v2:
> - Return error from check script if one of the make invocations returns
>  non-zero (even if binary is successfully produced).
> - Run "make clean" from bpf/ and not only bpf/bpftool/ in that same script,
>  when relevant.
> - Add a patch to clean up generated "feature/" and "libbpf/" directories.
> 
> Cc: Lorenz Bauer <lmb@cloudflare.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> 
> Quentin Monnet (4):
>  tools: bpftool: ignore make built-in rules for getting kernel version
>  tools: bpftool: improve and check builds for different make
>    invocations
>  tools: bpf: account for generated feature/ and libbpf/ directories
>  tools: bpftool: do not link twice against libbpf.a in Makefile
> 
> tools/bpf/.gitignore                          |   1 +
> tools/bpf/Makefile                            |   5 +-
> tools/bpf/bpftool/.gitignore                  |   2 +
> tools/bpf/bpftool/Makefile                    |  28 ++--
> tools/testing/selftests/bpf/Makefile          |   3 +-
> .../selftests/bpf/test_bpftool_build.sh       | 143 ++++++++++++++++++
> 6 files changed, 167 insertions(+), 15 deletions(-)
> create mode 100755 tools/testing/selftests/bpf/test_bpftool_build.sh
> 
> -- 
> 2.17.1

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>

for the whole series.

^ permalink raw reply

* Re: [PATCH][V2] wimax/i2400m: remove debug containing bogus calculation of index
From: Dan Carpenter @ 2019-08-30 11:40 UTC (permalink / raw)
  To: Colin King
  Cc: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190830090711.15300-1-colin.king@canonical.com>

On Fri, Aug 30, 2019 at 10:07:11AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The subtraction of the two pointers is automatically scaled by the
> size of the size of the object the pointers point to, so the division
> by sizeof(*i2400m->barker) is incorrect.  This has been broken since
> day one of the driver and is only debug, so remove the debug completely.
> 
> Also move && in condition to clean up a checkpatch warning.
> 
> Addresses-Coverity: ("Extra sizeof expression")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter


^ permalink raw reply

* [PATCH bpf-next v2 1/4] tools: bpftool: ignore make built-in rules for getting kernel version
From: Quentin Monnet @ 2019-08-30 11:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, oss-drivers, Quentin Monnet, Lorenz Bauer,
	Ilya Leoshkevich, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>

Bpftool calls the toplevel Makefile to get the kernel version for the
sources it is built from. But when the utility is built from the top of
the kernel repository, it may dump the following error message for
certain architectures (including x86):

    $ make tools/bpf
    [...]
    make[3]: *** [checkbin] Error 1
    [...]

This does not prevent bpftool compilation, but may feel disconcerting.
The "checkbin" arch-dependent target is not supposed to be called for
target "kernelversion", which is a simple "echo" of the version number.

It turns out this is caused by the make invocation in tools/bpf/bpftool,
which attempts to find implicit rules to apply. Extract from debug
output:

    Reading makefiles...
    Reading makefile 'Makefile'...
    Reading makefile 'scripts/Kbuild.include' (search path) (no ~ expansion)...
    Reading makefile 'scripts/subarch.include' (search path) (no ~ expansion)...
    Reading makefile 'arch/x86/Makefile' (search path) (no ~ expansion)...
    Reading makefile 'scripts/Makefile.kcov' (search path) (no ~ expansion)...
    Reading makefile 'scripts/Makefile.gcc-plugins' (search path) (no ~ expansion)...
    Reading makefile 'scripts/Makefile.kasan' (search path) (no ~ expansion)...
    Reading makefile 'scripts/Makefile.extrawarn' (search path) (no ~ expansion)...
    Reading makefile 'scripts/Makefile.ubsan' (search path) (no ~ expansion)...
    Updating makefiles....
     Considering target file 'scripts/Makefile.ubsan'.
      Looking for an implicit rule for 'scripts/Makefile.ubsan'.
      Trying pattern rule with stem 'Makefile.ubsan'.
    [...]
      Trying pattern rule with stem 'Makefile.ubsan'.
      Trying implicit prerequisite 'scripts/Makefile.ubsan.o'.
      Looking for a rule with intermediate file 'scripts/Makefile.ubsan.o'.
       Avoiding implicit rule recursion.
       Trying pattern rule with stem 'Makefile.ubsan'.
       Trying rule prerequisite 'prepare'.
       Trying rule prerequisite 'FORCE'.
      Found an implicit rule for 'scripts/Makefile.ubsan'.
        Considering target file 'prepare'.
         File 'prepare' does not exist.
          Considering target file 'prepare0'.
           File 'prepare0' does not exist.
            Considering target file 'archprepare'.
             File 'archprepare' does not exist.
              Considering target file 'archheaders'.
               File 'archheaders' does not exist.
               Finished prerequisites of target file 'archheaders'.
              Must remake target 'archheaders'.
    Putting child 0x55976f4f6980 (archheaders) PID 31743 on the chain.

To avoid that, pass the -r and -R flags to eliminate the use of make
built-in rules (and while at it, built-in variables) when running
command "make kernelversion" from bpftool's Makefile.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index f284c207765a..cd0fc05464e7 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -24,7 +24,7 @@ endif
 
 LIBBPF = $(BPF_PATH)libbpf.a
 
-BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion)
+BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 
 $(LIBBPF): FORCE
 	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next v2 4/4] tools: bpftool: do not link twice against libbpf.a in Makefile
From: Quentin Monnet @ 2019-08-30 11:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, oss-drivers, Quentin Monnet, Lorenz Bauer,
	Ilya Leoshkevich, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>

In bpftool's Makefile, $(LIBS) includes $(LIBBPF), therefore the library
is used twice in the linking command. No need to have $(LIBBPF) (from
$^) on that command, let's do with "$(OBJS) $(LIBS)" (but move $(LIBBPF)
_before_ the -l flags in $(LIBS)).

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index b0c5a369f54a..39bc6f0f4f0b 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -55,7 +55,7 @@ ifneq ($(EXTRA_LDFLAGS),)
 LDFLAGS += $(EXTRA_LDFLAGS)
 endif
 
-LIBS = -lelf -lz $(LIBBPF)
+LIBS = $(LIBBPF) -lelf -lz
 
 INSTALL ?= install
 RM ?= rm -f
@@ -117,7 +117,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
 $(OUTPUT)feature.o: | zdep
 
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
-	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
 
 $(OUTPUT)%.o: %.c
 	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next v2 3/4] tools: bpf: account for generated feature/ and libbpf/ directories
From: Quentin Monnet @ 2019-08-30 11:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, oss-drivers, Quentin Monnet, Lorenz Bauer,
	Ilya Leoshkevich, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>

When building "tools/bpf" from the top of the Linux repository, the
build system passes a value for the $(OUTPUT) Makefile variable to
tools/bpf/Makefile and tools/bpf/bpftool/Makefile, which results in
generating "libbpf/" (for bpftool) and "feature/" (bpf and bpftool)
directories inside the tree.

This commit adds such directories to the relevant .gitignore files, and
edits the Makefiles to ensure they are removed on "make clean". The use
of "rm" is also made consistent throughout those Makefiles (relies on
the $(RM) variable, use "--" to prevent interpreting
$(OUTPUT)/$(DESTDIR) as options.

v2:
- New patch.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/.gitignore         |  1 +
 tools/bpf/Makefile           |  5 +++--
 tools/bpf/bpftool/.gitignore |  2 ++
 tools/bpf/bpftool/Makefile   | 10 ++++++----
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
index dfe2bd5a4b95..59024197e71d 100644
--- a/tools/bpf/.gitignore
+++ b/tools/bpf/.gitignore
@@ -1,4 +1,5 @@
 FEATURE-DUMP.bpf
+feature
 bpf_asm
 bpf_dbg
 bpf_exp.yacc.*
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 53b60ad452f5..fbf5e4a0cb9c 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -81,10 +81,11 @@ $(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
 
 clean: bpftool_clean
 	$(call QUIET_CLEAN, bpf-progs)
-	$(Q)rm -rf $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
+	$(Q)$(RM) -r -- $(OUTPUT)*.o $(OUTPUT)bpf_jit_disasm $(OUTPUT)bpf_dbg \
 	       $(OUTPUT)bpf_asm $(OUTPUT)bpf_exp.yacc.* $(OUTPUT)bpf_exp.lex.*
 	$(call QUIET_CLEAN, core-gen)
-	$(Q)rm -f $(OUTPUT)FEATURE-DUMP.bpf
+	$(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpf
+	$(Q)$(RM) -r -- $(OUTPUT)feature
 
 install: $(PROGS) bpftool_install
 	$(call QUIET_INSTALL, bpf_jit_disasm)
diff --git a/tools/bpf/bpftool/.gitignore b/tools/bpf/bpftool/.gitignore
index 8248b8dd89d4..b13926432b84 100644
--- a/tools/bpf/bpftool/.gitignore
+++ b/tools/bpf/bpftool/.gitignore
@@ -3,3 +3,5 @@
 bpftool*.8
 bpf-helpers.*
 FEATURE-DUMP.bpftool
+feature
+libbpf
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 3fc82ff9b52c..b0c5a369f54a 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -124,9 +124,11 @@ $(OUTPUT)%.o: %.c
 
 clean: $(LIBBPF)-clean
 	$(call QUIET_CLEAN, bpftool)
-	$(Q)$(RM) $(OUTPUT)bpftool $(OUTPUT)*.o $(OUTPUT)*.d
+	$(Q)$(RM) -- $(OUTPUT)bpftool $(OUTPUT)*.o $(OUTPUT)*.d
+	$(Q)$(RM) -r -- $(OUTPUT)libbpf/
 	$(call QUIET_CLEAN, core-gen)
-	$(Q)$(RM) $(OUTPUT)FEATURE-DUMP.bpftool
+	$(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpftool
+	$(Q)$(RM) -r -- $(OUTPUT)feature/
 
 install: $(OUTPUT)bpftool
 	$(call QUIET_INSTALL, bpftool)
@@ -137,8 +139,8 @@ install: $(OUTPUT)bpftool
 
 uninstall:
 	$(call QUIET_UNINST, bpftool)
-	$(Q)$(RM) $(DESTDIR)$(prefix)/sbin/bpftool
-	$(Q)$(RM) $(DESTDIR)$(bash_compdir)/bpftool
+	$(Q)$(RM) -- $(DESTDIR)$(prefix)/sbin/bpftool
+	$(Q)$(RM) -- $(DESTDIR)$(bash_compdir)/bpftool
 
 doc:
 	$(call descend,Documentation)
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next v2 2/4] tools: bpftool: improve and check builds for different make invocations
From: Quentin Monnet @ 2019-08-30 11:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, oss-drivers, Quentin Monnet, Lorenz Bauer,
	Ilya Leoshkevich, Jakub Kicinski
In-Reply-To: <20190830110040.31257-1-quentin.monnet@netronome.com>

There are a number of alternative "make" invocations that can be used to
compile bpftool. The following invocations are expected to work:

  - through the kbuild system, from the top of the repository
    (make tools/bpf)
  - by telling make to change to the bpftool directory
    (make -C tools/bpf/bpftool)
  - by building the BPF tools from tools/
    (cd tools && make bpf)
  - by running make from bpftool directory
    (cd tools/bpf/bpftool && make)

Additionally, setting the O or OUTPUT variables should tell the build
system to use a custom output path, for each of these alternatives.

The following patch fixes the following invocations:

  $ make tools/bpf
  $ make tools/bpf O=<dir>
  $ make -C tools/bpf/bpftool OUTPUT=<dir>
  $ make -C tools/bpf/bpftool O=<dir>
  $ cd tools/ && make bpf O=<dir>
  $ cd tools/bpf/bpftool && make OUTPUT=<dir>
  $ cd tools/bpf/bpftool && make O=<dir>

After this commit, the build still fails for two variants when passing
the OUTPUT variable:

  $ make tools/bpf OUTPUT=<dir>
  $ cd tools/ && make bpf OUTPUT=<dir>

In order to remember and check what make invocations are supposed to
work, and to document the ones which do not, a new script is added to
the BPF selftests. Note that some invocations require the kernel to be
configured, so the script skips them if no .config file is found.

v2:
- In make_and_clean(), set $ERROR to 1 when "make" returns non-zero,
  even if the binary was produced.
- Run "make clean" from the correct directory (bpf/ instead of bpftool/,
  when relevant).

Reported-by: Lorenz Bauer <lmb@cloudflare.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Makefile                    |  12 +-
 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../selftests/bpf/test_bpftool_build.sh       | 143 ++++++++++++++++++
 3 files changed, 152 insertions(+), 6 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_bpftool_build.sh

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index cd0fc05464e7..3fc82ff9b52c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -17,21 +17,23 @@ endif
 BPF_DIR = $(srctree)/tools/lib/bpf/
 
 ifneq ($(OUTPUT),)
-  BPF_PATH = $(OUTPUT)
+  LIBBPF_OUTPUT = $(OUTPUT)/libbpf/
+  LIBBPF_PATH = $(LIBBPF_OUTPUT)
 else
-  BPF_PATH = $(BPF_DIR)
+  LIBBPF_PATH = $(BPF_DIR)
 endif
 
-LIBBPF = $(BPF_PATH)libbpf.a
+LIBBPF = $(LIBBPF_PATH)libbpf.a
 
 BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 
 $(LIBBPF): FORCE
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a
+	$(if $(LIBBPF_OUTPUT),@mkdir -p $(LIBBPF_OUTPUT))
+	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) $(LIBBPF_OUTPUT)libbpf.a
 
 $(LIBBPF)-clean:
 	$(call QUIET_CLEAN, libbpf)
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) clean >/dev/null
+	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) clean >/dev/null
 
 prefix ?= /usr/local
 bash_compdir ?= /usr/share/bash-completion/completions
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1faad0c3c3c9..c7595b4ed55d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -63,7 +63,8 @@ TEST_PROGS := test_kmod.sh \
 	test_tcp_check_syncookie.sh \
 	test_tc_tunnel.sh \
 	test_tc_edt.sh \
-	test_xdping.sh
+	test_xdping.sh \
+	test_bpftool_build.sh
 
 TEST_PROGS_EXTENDED := with_addr.sh \
 	with_tunnels.sh \
diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
new file mode 100755
index 000000000000..4ba5a34bff56
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -0,0 +1,143 @@
+#!/bin/bash
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+ERROR=0
+TMPDIR=
+
+# If one build fails, continue but return non-0 on exit.
+return_value() {
+	if [ -d "$TMPDIR" ] ; then
+		rm -rf -- $TMPDIR
+	fi
+	exit $ERROR
+}
+trap return_value EXIT
+
+case $1 in
+	-h|--help)
+		echo -e "$0 [-j <n>]"
+		echo -e "\tTest the different ways of building bpftool."
+		echo -e ""
+		echo -e "\tOptions:"
+		echo -e "\t\t-j <n>:\tPass -j flag to 'make'."
+		exit
+		;;
+esac
+
+J=$*
+
+# Assume script is located under tools/testing/selftests/bpf/. We want to start
+# build attempts from the top of kernel repository.
+SCRIPT_REL_PATH=$(realpath --relative-to=$PWD $0)
+SCRIPT_REL_DIR=$(dirname $SCRIPT_REL_PATH)
+KDIR_ROOT_DIR=$(realpath $PWD/$SCRIPT_REL_DIR/../../../../)
+cd $KDIR_ROOT_DIR
+
+check() {
+	local dir=$(realpath $1)
+
+	echo -n "binary:  "
+	# Returns non-null if file is found (and "false" is run)
+	find $dir -type f -executable -name bpftool -print -exec false {} + && \
+		ERROR=1 && printf "FAILURE: Did not find bpftool\n"
+}
+
+make_and_clean() {
+	echo -e "\$PWD:    $PWD"
+	echo -e "command: make -s $* >/dev/null"
+	make $J -s $* >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+	if [ $# -ge 1 ] ; then
+		check ${@: -1}
+	else
+		check .
+	fi
+	(
+		if [ $# -ge 1 ] ; then
+			cd ${@: -1}
+		fi
+		make -s clean
+	)
+	echo
+}
+
+make_with_tmpdir() {
+	local ARGS
+
+	TMPDIR=$(mktemp -d)
+	if [ $# -ge 2 ] ; then
+		ARGS=${@:1:(($# - 1))}
+	fi
+	echo -e "\$PWD:    $PWD"
+	echo -e "command: make -s $ARGS ${@: -1}=$TMPDIR/ >/dev/null"
+	make $J -s $ARGS ${@: -1}=$TMPDIR/ >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+	check $TMPDIR
+	rm -rf -- $TMPDIR
+	echo
+}
+
+echo "Trying to build bpftool"
+echo -e "... through kbuild\n"
+
+if [ -f ".config" ] ; then
+	make_and_clean tools/bpf
+
+	## $OUTPUT is overwritten in kbuild Makefile, and thus cannot be passed
+	## down from toplevel Makefile to bpftool's Makefile.
+
+	# make_with_tmpdir tools/bpf OUTPUT
+	echo -e "skip:    make tools/bpf OUTPUT=<dir> (not supported)\n"
+
+	make_with_tmpdir tools/bpf O
+else
+	echo -e "skip:    make tools/bpf (no .config found)\n"
+	echo -e "skip:    make tools/bpf OUTPUT=<dir> (not supported)\n"
+	echo -e "skip:    make tools/bpf O=<dir> (no .config found)\n"
+fi
+
+echo -e "... from kernel source tree\n"
+
+make_and_clean -C tools/bpf/bpftool
+
+make_with_tmpdir -C tools/bpf/bpftool OUTPUT
+
+make_with_tmpdir -C tools/bpf/bpftool O
+
+echo -e "... from tools/\n"
+cd tools/
+
+make_and_clean bpf
+
+## In tools/bpf/Makefile, function "descend" is called and passes $(O) and
+## $(OUTPUT). We would like $(OUTPUT) to have "bpf/bpftool/" appended before
+## calling bpftool's Makefile, but this is not the case as the "descend"
+## function focuses on $(O)/$(subdir). However, in the present case, updating
+## $(O) to have $(OUTPUT) recomputed from it in bpftool's Makefile does not
+## work, because $(O) is not defined from command line and $(OUTPUT) is not
+## updated in tools/scripts/Makefile.include.
+##
+## Workarounds would require to a) edit "descend" or use an alternative way to
+## call bpftool's Makefile, b) modify the conditions to update $(OUTPUT) and
+## other variables in tools/scripts/Makefile.include (at the risk of breaking
+## the build of other tools), or c) append manually the "bpf/bpftool" suffix to
+## $(OUTPUT) in bpf's Makefile, which may break if targets for other directories
+## use "descend" in the future.
+
+# make_with_tmpdir bpf OUTPUT
+echo -e "skip:    make bpf OUTPUT=<dir> (not supported)\n"
+
+make_with_tmpdir bpf O
+
+echo -e "... from bpftool's dir\n"
+cd bpf/bpftool
+
+make_and_clean
+
+make_with_tmpdir OUTPUT
+
+make_with_tmpdir O
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next v2 0/4] tools: bpftool: improve bpftool build experience
From: Quentin Monnet @ 2019-08-30 11:00 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, oss-drivers, Quentin Monnet, Lorenz Bauer,
	Ilya Leoshkevich, Jakub Kicinski

Hi,
This set attempts to make it easier to build bpftool, in particular when
passing a specific output directory. This is a follow-up to the
conversation held last month by Lorenz, Ilya and Jakub [0].

The first patch is a minor fix to bpftool's Makefile, regarding the
retrieval of kernel version (which currently prints a non-relevant make
warning on some invocations).

Second patch improves the Makefile commands to support more "make"
invocations, or to fix building with custom output directory. On Jakub's
suggestion, a script is also added to BPF selftests in order to keep track
of the supported build variants.

Building bpftool with "make tools/bpf" from the top of the repository
generates files in "libbpf/" and "feature/" directories under tools/bpf/
and tools/bpf/bpftool/. The third patch ensures such directories are taken
care of on "make clean", and add them to the relevant .gitignore files.

At last, fourth patch is a sligthly modified version of Ilya's fix
regarding libbpf.a appearing twice on the linking command for bpftool.

[0] https://lore.kernel.org/bpf/CACAyw9-CWRHVH3TJ=Tke2x8YiLsH47sLCijdp=V+5M836R9aAA@mail.gmail.com/

v2:
- Return error from check script if one of the make invocations returns
  non-zero (even if binary is successfully produced).
- Run "make clean" from bpf/ and not only bpf/bpftool/ in that same script,
  when relevant.
- Add a patch to clean up generated "feature/" and "libbpf/" directories.

Cc: Lorenz Bauer <lmb@cloudflare.com>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>

Quentin Monnet (4):
  tools: bpftool: ignore make built-in rules for getting kernel version
  tools: bpftool: improve and check builds for different make
    invocations
  tools: bpf: account for generated feature/ and libbpf/ directories
  tools: bpftool: do not link twice against libbpf.a in Makefile

 tools/bpf/.gitignore                          |   1 +
 tools/bpf/Makefile                            |   5 +-
 tools/bpf/bpftool/.gitignore                  |   2 +
 tools/bpf/bpftool/Makefile                    |  28 ++--
 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../selftests/bpf/test_bpftool_build.sh       | 143 ++++++++++++++++++
 6 files changed, 167 insertions(+), 15 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_bpftool_build.sh

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH bpf-next 2/3] tools: bpftool: improve and check builds for different make invocations
From: Quentin Monnet @ 2019-08-30 10:58 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev, oss-drivers,
	Lorenz Bauer, Jakub Kicinski
In-Reply-To: <C90F7A80-D2E9-401B-8BFC-47C22A44ADAC@linux.ibm.com>

2019-08-29 18:03 UTC+0200 ~ Ilya Leoshkevich <iii@linux.ibm.com>
>> Am 29.08.2019 um 12:56 schrieb Quentin Monnet <quentin.monnet@netronome.com>:
>>
>> +make_and_clean() {
>> +	echo -e "\$PWD:    $PWD"
>> +	echo -e "command: make -s $* >/dev/null"
>> +	make $J -s $* >/dev/null
> 
> Would it make sense to set ERROR=1 if make produces a bpftool binary,
> but still fails with a non-zero RC for whatever reason?
> 

Hi Ilya,

Generating bpftool being the last thing the Makefile does, I don't know
if this could happen. But sure, that wouldn't hurt, and I will add it to
v2, thanks!

Quentin

^ permalink raw reply

* [PATCH net-next 2/2] devlink: Use switch-case instead of if-else
From: Parav Pandit @ 2019-08-30 10:39 UTC (permalink / raw)
  To: netdev, davem; +Cc: jiri, Parav Pandit
In-Reply-To: <20190830103945.18097-1-parav@mellanox.com>

Make core more readable with switch-case for various port flavours.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 net/core/devlink.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index b7091329987a..6e52d639dac6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -510,32 +510,37 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		return 0;
 	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
 		return -EMSGSIZE;
-	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+	switch (devlink_port->attrs.flavour) {
+	case DEVLINK_PORT_FLAVOUR_PCI_PF:
 		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
 				attrs->pci_pf.pf))
 			return -EMSGSIZE;
-	} else if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+		break;
+	case DEVLINK_PORT_FLAVOUR_PCI_VF:
 		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
 				attrs->pci_vf.pf) ||
 		    nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_VF_NUMBER,
 				attrs->pci_vf.vf))
 			return -EMSGSIZE;
+		break;
+	case DEVLINK_PORT_FLAVOUR_PHYSICAL:
+	case DEVLINK_PORT_FLAVOUR_CPU:
+	case DEVLINK_PORT_FLAVOUR_DSA:
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
+				attrs->phys.port_number))
+			return -EMSGSIZE;
+		if (!attrs->split)
+			return 0;
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
+				attrs->phys.port_number))
+			return -EMSGSIZE;
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
+				attrs->phys.split_subport_number))
+			return -EMSGSIZE;
+		break;
+	default:
+		break;
 	}
-	if (devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_PHYSICAL &&
-	    devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_CPU &&
-	    devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_DSA)
-		return 0;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
-			attrs->phys.port_number))
-		return -EMSGSIZE;
-	if (!attrs->split)
-		return 0;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
-			attrs->phys.port_number))
-		return -EMSGSIZE;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
-			attrs->phys.split_subport_number))
-		return -EMSGSIZE;
 	return 0;
 }
 
-- 
2.19.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox