Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Jakub Kicinski @ 2019-02-25 18:47 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Jesper Dangaard Brouer, David Miller, netdev, Daniel Borkmann,
	Alexei Starovoitov
In-Reply-To: <875ztawvqh.fsf@toke.dk>

On Sat, 23 Feb 2019 13:11:02 +0100, Toke Høiland-Jørgensen wrote:
> Jesper Dangaard Brouer <brouer@redhat.com> writes:
> > On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski wrote:
> >> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:  
> >> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:    
> >> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:      
> > [...]  
> >> > >
> >> > > BPF programs don't obey by netns boundaries.  The fact the program is
> >> > > verified in one ns doesn't mean this is the only ns it will be used in :(
> >> > > Meaning if any program is using the redirect map you may need a secret
> >> > > map in every ns.. no?      
> >> > 
> >> > Ah, yes, good point. Totally didn't think about the fact that load and
> >> > attach are decoupled. Hmm, guess I'll just have to move the call to
> >> > alloc_default_map() to the point where the program is attached to an
> >> > interface, then...    
> >> 
> >> Possibly.. and you also need to handle the case where interface with a
> >> program attached is moved, no?  
> 
> Yup, alloc on attach was easy enough; the moving turns out to be the
> tricky part :)
> 
> > True, we need to handle if e.g. a veth gets an XDP program attached and
> > then is moved into a network namespace (as I've already explained to
> > Toke in a meeting).  
> 
> Yeah, I had somehow convinced myself that the XDP program was being
> removed when the interface was being torn down before moving between
> namespaces. Jesper pointed out that this was not in fact the case... :P
> 
> > I'm still not sure how to handle this...  
> 
> There are a couple of options, I think. At least:
> 
> 1. Maintain a flag on struct net_device indicating that this device
>    needs the redirect map allocated, and react to that when interfaces
>    are being moved.
> 
> 2. Lookup the BPF program by ID (which we can get from the driver) on
>    move, and react to the program flag.
> 
> 3. Keep the allocation on program load, but allocate maps for all active
>    namespaces (which would probably need a refcnt mechanism to
>    deallocate things again).
> 
> I think I'm leaning towards #2; possibly combined with a refcnt so we
> can actually deallocate the map in the root namespace when it's not
> needed anymore.

Okay.. what about tail calls?  I think #3 is most reasonable complexity-
-wise, or some mix of #2 and #3 - cnt the programs with legacy
redirects, and then allocate the resources if cnt && name space has any
XDP program attached.

Can users really not be told to just use the correct helper? ;)

^ permalink raw reply

* Re: [RFC v1 12/19] RDMA/irdma: Implement device supported verb APIs
From: Jason Gunthorpe @ 2019-02-25 18:50 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Shiraz Saleem, dledford, davem, linux-rdma, netdev,
	mustafa.ismail, jeffrey.t.kirsher, Yossi Leybovich
In-Reply-To: <01b0d571-81d8-ed6c-77b7-e83ee0ab9caa@amazon.com>

On Sun, Feb 24, 2019 at 04:35:02PM +0200, Gal Pressman wrote:
> > +/**> + * irdma_disassociate_ucontext - Disassociate user context> + * @context: ib user context> + */> +static void irdma_disassociate_ucontext(struct ib_ucontext *context)
> > +{
> > +}
> 
> What's the motivation for a nop callback (over not implementing the
> function)?

This is my fault, I didn't finish yet and conver disassociate_ucontext
into a flags once they were all made empty.

> > +	ret = irdma_alloc_rsrc(iwdev->rf,
> > +			       iwdev->rf->allocated_mrs, iwdev->rf->max_mr,
> > +			       &stag_index, &next_stag_index);
> > +	if (!ret) {
> > +		stag = stag_index << IRDMA_CQPSQ_STAG_IDX_S;
> > +		stag |= driver_key;
> > +		stag += (u32)consumer_key;
> > +		irdma_add_devusecount(iwdev);
> > +	}
> 
> This is confusing IMHO, better to test for 'if (ret)' and keep the main flow
> unindented.

Yes please follow the standard 'success oriented flow'

Jason

^ permalink raw reply

* [PATCH net-next v3 0/2] net: phy: aquantia: add hwmon support
From: Heiner Kallweit @ 2019-02-25 18:52 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

This series adds HWMON support for the temperature sensor and the
related alarms on the 107/108/109 chips.

v2:
- remove struct aqr_priv
- rename header file to aquantia.h
v3:
- add conditional compiling of aquantia_hwmon.c
- improve converting sensor register values to/from long
- add helper aqr_hwmon_test_bit

Heiner Kallweit (2):
  net: phy: aquantia: rename aquantia.c to aquantia_main.c
  net: phy: aquantia: add hwmon support

 drivers/net/phy/Makefile                      |   4 +
 drivers/net/phy/aquantia.h                    |  16 ++
 drivers/net/phy/aquantia_hwmon.c              | 250 ++++++++++++++++++
 .../net/phy/{aquantia.c => aquantia_main.c}   |   4 +
 4 files changed, 274 insertions(+)
 create mode 100644 drivers/net/phy/aquantia.h
 create mode 100644 drivers/net/phy/aquantia_hwmon.c
 rename drivers/net/phy/{aquantia.c => aquantia_main.c} (99%)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next v3 1/2] net: phy: aquantia: rename aquantia.c to aquantia_main.c
From: Heiner Kallweit @ 2019-02-25 18:53 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <71142110-f853-86e8-e597-f205d97d3491@gmail.com>

Rename aquantia.c to aquantia_main.c to be prepared for adding new
functionality to separate source code files.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/Makefile                        | 1 +
 drivers/net/phy/{aquantia.c => aquantia_main.c} | 0
 2 files changed, 1 insertion(+)
 rename drivers/net/phy/{aquantia.c => aquantia_main.c} (100%)

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 5805c0b7d..b0845adaf 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -45,6 +45,7 @@ sfp-obj-$(CONFIG_SFP)		+= sfp-bus.o
 obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
 
 obj-$(CONFIG_AMD_PHY)		+= amd.o
+aquantia-objs			+= aquantia_main.o
 obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
 obj-$(CONFIG_ASIX_PHY)		+= asix.o
 obj-$(CONFIG_AT803X_PHY)	+= at803x.o
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia_main.c
similarity index 100%
rename from drivers/net/phy/aquantia.c
rename to drivers/net/phy/aquantia_main.c
-- 
2.20.1



^ permalink raw reply related

* [PATCH net-next v3 2/2] net: phy: aquantia: add hwmon support
From: Heiner Kallweit @ 2019-02-25 18:56 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <71142110-f853-86e8-e597-f205d97d3491@gmail.com>

This adds HWMON support for the temperature sensor and the related
alarms on the 107/108/109 chips. This patch is based on work from
Nikita and Andrew. I added:
- support for changing alarm thresholds via sysfs
- move HWMON code to a separate source file to improve maintainability
- smaller changes like using IS_REACHABLE instead of ifdef
  (avoids problems if PHY driver is built in and HWMON is a module)

v2:
- remove struct aqr_priv
- rename header file to aquantia.h
v3:
- add conditional compiling of aquantia_hwmon.c
- improve converting sensor register values to/from long
- add helper aqr_hwmon_test_bit

Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/Makefile         |   3 +
 drivers/net/phy/aquantia.h       |  16 ++
 drivers/net/phy/aquantia_hwmon.c | 250 +++++++++++++++++++++++++++++++
 drivers/net/phy/aquantia_main.c  |   4 +
 4 files changed, 273 insertions(+)
 create mode 100644 drivers/net/phy/aquantia.h
 create mode 100644 drivers/net/phy/aquantia_hwmon.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index b0845adaf..73e9670ef 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -46,6 +46,9 @@ obj-y				+= $(sfp-obj-y) $(sfp-obj-m)
 
 obj-$(CONFIG_AMD_PHY)		+= amd.o
 aquantia-objs			+= aquantia_main.o
+ifdef CONFIG_HWMON
+aquantia-objs			+= aquantia_hwmon.o
+endif
 obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
 obj-$(CONFIG_ASIX_PHY)		+= asix.o
 obj-$(CONFIG_AT803X_PHY)	+= at803x.o
diff --git a/drivers/net/phy/aquantia.h b/drivers/net/phy/aquantia.h
new file mode 100644
index 000000000..5a16caab7
--- /dev/null
+++ b/drivers/net/phy/aquantia.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * HWMON driver for Aquantia PHY
+ *
+ * Author: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
+ * Author: Andrew Lunn <andrew@lunn.ch>
+ * Author: Heiner Kallweit <hkallweit1@gmail.com>
+ */
+
+#include <linux/device.h>
+#include <linux/phy.h>
+
+#if IS_REACHABLE(CONFIG_HWMON)
+int aqr_hwmon_probe(struct phy_device *phydev);
+#else
+static inline int aqr_hwmon_probe(struct phy_device *phydev) { return 0; }
+#endif
diff --git a/drivers/net/phy/aquantia_hwmon.c b/drivers/net/phy/aquantia_hwmon.c
new file mode 100644
index 000000000..19c4c280a
--- /dev/null
+++ b/drivers/net/phy/aquantia_hwmon.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/* HWMON driver for Aquantia PHY
+ *
+ * Author: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
+ * Author: Andrew Lunn <andrew@lunn.ch>
+ * Author: Heiner Kallweit <hkallweit1@gmail.com>
+ */
+
+#include <linux/phy.h>
+#include <linux/device.h>
+#include <linux/ctype.h>
+#include <linux/hwmon.h>
+
+#include "aquantia.h"
+
+/* Vendor specific 1, MDIO_MMD_VEND2 */
+#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL	0xc421
+#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL	0xc422
+#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN	0xc423
+#define VEND1_THERMAL_PROV_LOW_TEMP_WARN	0xc424
+#define VEND1_THERMAL_STAT1			0xc820
+#define VEND1_THERMAL_STAT2			0xc821
+#define VEND1_THERMAL_STAT2_VALID		BIT(0)
+#define VEND1_GENERAL_STAT1			0xc830
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL	BIT(14)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL	BIT(13)
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN	BIT(12)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN	BIT(11)
+
+#if IS_REACHABLE(CONFIG_HWMON)
+
+static umode_t aqr_hwmon_is_visible(const void *data,
+				    enum hwmon_sensor_types type,
+				    u32 attr, int channel)
+{
+	if (type != hwmon_temp)
+		return 0;
+
+	switch (attr) {
+	case hwmon_temp_input:
+	case hwmon_temp_min_alarm:
+	case hwmon_temp_max_alarm:
+	case hwmon_temp_lcrit_alarm:
+	case hwmon_temp_crit_alarm:
+		return 0444;
+	case hwmon_temp_min:
+	case hwmon_temp_max:
+	case hwmon_temp_lcrit:
+	case hwmon_temp_crit:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
+static int aqr_hwmon_get(struct phy_device *phydev, int reg, long *value)
+{
+	int temp = phy_read_mmd(phydev, MDIO_MMD_VEND1, reg);
+
+	if (temp < 0)
+		return temp;
+
+	/* 16 bit value is 2's complement with LSB = 1/256th degree Celsius */
+	*value = (s16)temp * 1000 / 256;
+
+	return 0;
+}
+
+static int aqr_hwmon_set(struct phy_device *phydev, int reg, long value)
+{
+	int temp;
+
+	if (value >= 128000 || value < -128000)
+		return -ERANGE;
+
+	temp = value * 256 / 1000;
+
+	/* temp is in s16 range and we're interested in lower 16 bits only */
+	return phy_write_mmd(phydev, MDIO_MMD_VEND1, reg, (u16)temp);
+}
+
+static int aqr_hwmon_test_bit(struct phy_device *phydev, int reg, int bit)
+{
+	int val = phy_read_mmd(phydev, MDIO_MMD_VEND1, reg);
+
+	if (val < 0)
+		return val;
+
+	return !!(val & bit);
+}
+
+static int aqr_hwmon_status1(struct phy_device *phydev, int bit, long *value)
+{
+	int val = aqr_hwmon_test_bit(phydev, VEND1_GENERAL_STAT1, bit);
+
+	if (val < 0)
+		return val;
+
+	*value = val;
+
+	return 0;
+}
+
+static int aqr_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			  u32 attr, int channel, long *value)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	int reg;
+
+	if (type != hwmon_temp)
+		return -EOPNOTSUPP;
+
+	switch (attr) {
+	case hwmon_temp_input:
+		reg = aqr_hwmon_test_bit(phydev, VEND1_THERMAL_STAT2,
+					 VEND1_THERMAL_STAT2_VALID);
+		if (reg < 0)
+			return reg;
+		if (!reg)
+			return -EBUSY;
+
+		return aqr_hwmon_get(phydev, VEND1_THERMAL_STAT1, value);
+
+	case hwmon_temp_lcrit:
+		return aqr_hwmon_get(phydev, VEND1_THERMAL_PROV_LOW_TEMP_FAIL,
+				     value);
+	case hwmon_temp_min:
+		return aqr_hwmon_get(phydev, VEND1_THERMAL_PROV_LOW_TEMP_WARN,
+				     value);
+	case hwmon_temp_max:
+		return aqr_hwmon_get(phydev, VEND1_THERMAL_PROV_HIGH_TEMP_WARN,
+				     value);
+	case hwmon_temp_crit:
+		return aqr_hwmon_get(phydev, VEND1_THERMAL_PROV_HIGH_TEMP_FAIL,
+				     value);
+	case hwmon_temp_lcrit_alarm:
+		return aqr_hwmon_status1(phydev,
+					 VEND1_GENERAL_STAT1_LOW_TEMP_FAIL,
+					 value);
+	case hwmon_temp_min_alarm:
+		return aqr_hwmon_status1(phydev,
+					 VEND1_GENERAL_STAT1_LOW_TEMP_WARN,
+					 value);
+	case hwmon_temp_max_alarm:
+		return aqr_hwmon_status1(phydev,
+					 VEND1_GENERAL_STAT1_HIGH_TEMP_WARN,
+					 value);
+	case hwmon_temp_crit_alarm:
+		return aqr_hwmon_status1(phydev,
+					 VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL,
+					 value);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int aqr_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, long value)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+
+	if (type != hwmon_temp)
+		return -EOPNOTSUPP;
+
+	switch (attr) {
+	case hwmon_temp_lcrit:
+		return aqr_hwmon_set(phydev, VEND1_THERMAL_PROV_LOW_TEMP_FAIL,
+				     value);
+	case hwmon_temp_min:
+		return aqr_hwmon_set(phydev, VEND1_THERMAL_PROV_LOW_TEMP_WARN,
+				     value);
+	case hwmon_temp_max:
+		return aqr_hwmon_set(phydev, VEND1_THERMAL_PROV_HIGH_TEMP_WARN,
+				     value);
+	case hwmon_temp_crit:
+		return aqr_hwmon_set(phydev, VEND1_THERMAL_PROV_HIGH_TEMP_FAIL,
+				     value);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static const struct hwmon_ops aqr_hwmon_ops = {
+	.is_visible = aqr_hwmon_is_visible,
+	.read = aqr_hwmon_read,
+	.write = aqr_hwmon_write,
+};
+
+static u32 aqr_hwmon_chip_config[] = {
+	HWMON_C_REGISTER_TZ,
+	0,
+};
+
+static const struct hwmon_channel_info aqr_hwmon_chip = {
+	.type = hwmon_chip,
+	.config = aqr_hwmon_chip_config,
+};
+
+static u32 aqr_hwmon_temp_config[] = {
+	HWMON_T_INPUT |
+	HWMON_T_MAX | HWMON_T_MIN |
+	HWMON_T_MAX_ALARM | HWMON_T_MIN_ALARM |
+	HWMON_T_CRIT | HWMON_T_LCRIT |
+	HWMON_T_CRIT_ALARM | HWMON_T_LCRIT_ALARM,
+	0,
+};
+
+static const struct hwmon_channel_info aqr_hwmon_temp = {
+	.type = hwmon_temp,
+	.config = aqr_hwmon_temp_config,
+};
+
+static const struct hwmon_channel_info *aqr_hwmon_info[] = {
+	&aqr_hwmon_chip,
+	&aqr_hwmon_temp,
+	NULL,
+};
+
+static const struct hwmon_chip_info aqr_hwmon_chip_info = {
+	.ops = &aqr_hwmon_ops,
+	.info = aqr_hwmon_info,
+};
+
+int aqr_hwmon_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device *hwmon_dev;
+	char *hwmon_name;
+	int i, j;
+
+	hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
+	if (!hwmon_name)
+		return -ENOMEM;
+
+	for (i = j = 0; hwmon_name[i]; i++) {
+		if (isalnum(hwmon_name[i])) {
+			if (i != j)
+				hwmon_name[j] = hwmon_name[i];
+			j++;
+		}
+	}
+	hwmon_name[j] = '\0';
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, hwmon_name,
+					phydev, &aqr_hwmon_chip_info, NULL);
+
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+#endif
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 0f0eb5682..37218e5d7 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -12,6 +12,8 @@
 #include <linux/delay.h>
 #include <linux/phy.h>
 
+#include "aquantia.h"
+
 #define PHY_ID_AQ1202	0x03a1b445
 #define PHY_ID_AQ2104	0x03a1b460
 #define PHY_ID_AQR105	0x03a1b4a2
@@ -231,6 +233,7 @@ static struct phy_driver aqr_driver[] = {
 	.name		= "Aquantia AQR107",
 	.aneg_done	= genphy_c45_aneg_done,
 	.get_features	= genphy_c45_pma_read_abilities,
+	.probe		= aqr_hwmon_probe,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr	= aqr_config_intr,
 	.ack_interrupt	= aqr_ack_interrupt,
@@ -241,6 +244,7 @@ static struct phy_driver aqr_driver[] = {
 	.name		= "Aquantia AQCS109",
 	.aneg_done	= genphy_c45_aneg_done,
 	.get_features	= genphy_c45_pma_read_abilities,
+	.probe		= aqr_hwmon_probe,
 	.config_init	= aqcs109_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr	= aqr_config_intr,
-- 
2.20.1



^ permalink raw reply related

* Re: [PATCH net-next v3 1/2] net: phy: aquantia: rename aquantia.c to aquantia_main.c
From: Florian Fainelli @ 2019-02-25 18:59 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <63bd4e13-d81f-2285-358a-11e596a061b1@gmail.com>

On 2/25/19 10:53 AM, Heiner Kallweit wrote:
> Rename aquantia.c to aquantia_main.c to be prepared for adding new
> functionality to separate source code files.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-02-25 19:01 UTC (permalink / raw)
  To: Jiri Pirko, Ido Schimmel, David S. Miller
  Cc: netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL)

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)

Notice that, in this case, variable alloc_size is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
index df78d23b3ec3..cb3e663b1d37 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c
@@ -236,12 +236,10 @@ mlxsw_afk_key_info_create(struct mlxsw_afk *mlxsw_afk,
 			  struct mlxsw_afk_element_usage *elusage)
 {
 	struct mlxsw_afk_key_info *key_info;
-	size_t alloc_size;
 	int err;
 
-	alloc_size = sizeof(*key_info) +
-		     sizeof(key_info->blocks[0]) * mlxsw_afk->max_blocks;
-	key_info = kzalloc(alloc_size, GFP_KERNEL);
+	key_info = kzalloc(struct_size(key_info, blocks, mlxsw_afk->max_blocks),
+			   GFP_KERNEL);
 	if (!key_info)
 		return ERR_PTR(-ENOMEM);
 	err = mlxsw_afk_picker(mlxsw_afk, key_info, elusage);
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Leslie Monis, Mohit P . Tahiliani, Dave Taht,
	Jamal Hadi Salim

The current implementation of the PIE queuing discipline is according to the
IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
[PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
However, a lot of necessary modifications and enhancements have been proposed
in RFC 8033, which have not yet been incorporated in the source code of Linux.
This patch series helps in achieving the same.

Performance tests carried out using Flent [https://flent.org/]

Changes from v2 to v3:
  - Used div_u64() instead of direct division after explicit type casting as
    recommended by David

Changes from v1 to v2:
  - Excluded the patch setting PIE dynamically active/inactive as the test
    results were unsatisfactory
  - Fixed a scaling issue when adding more auto-tuning cases which caused
    local variables to underflow
  - Changed the long if/else chain to a loop as suggested by Stephen
  - Changed the position of the accu_prob variable in the pie_vars
    structure as recommended by Stephen

Mohit P. Tahiliani (7):
  net: sched: pie: change value of QUEUE_THRESHOLD
  net: sched: pie: change default value of pie_params->target
  net: sched: pie: change default value of pie_params->tupdate
  net: sched: pie: change initial value of pie_vars->burst_time
  net: sched: pie: add more cases to auto-tune alpha and beta
  net: sched: pie: add derandomization mechanism
  net: sched: pie: update references

 include/uapi/linux/pkt_sched.h |   2 +-
 net/sched/sch_pie.c            | 107 ++++++++++++++++++++-------------
 2 files changed, 66 insertions(+), 43 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH net-next v3 1/7] net: sched: pie: change value of QUEUE_THRESHOLD
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 recommends a value of 16384 bytes for the queue
threshold.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d1429371592f..7778eff6cdb7 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -31,7 +31,7 @@
 #include <net/pkt_sched.h>
 #include <net/inet_ecn.h>
 
-#define QUEUE_THRESHOLD 10000
+#define QUEUE_THRESHOLD 16384
 #define DQCOUNT_INVALID -1
 #define MAX_PROB  0xffffffff
 #define PIE_SCALE 8
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 2/7] net: sched: pie: change default value of pie_params->target
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests a default value of 15 milliseconds for the
target queue delay.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 7778eff6cdb7..91af9bf19852 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -83,7 +83,7 @@ static void pie_params_init(struct pie_params *params)
 	params->beta = 20;
 	params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC);	/* 30 ms */
 	params->limit = 1000;	/* default of 1000 packets */
-	params->target = PSCHED_NS2TICKS(20 * NSEC_PER_MSEC);	/* 20 ms */
+	params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC);	/* 15 ms */
 	params->ecn = false;
 	params->bytemode = false;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 3/7] net: sched: pie: change default value of pie_params->tupdate
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests a default value of 15 milliseconds for the
update interval.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 91af9bf19852..702f75afc312 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -81,7 +81,7 @@ static void pie_params_init(struct pie_params *params)
 {
 	params->alpha = 2;
 	params->beta = 20;
-	params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC);	/* 30 ms */
+	params->tupdate = usecs_to_jiffies(15 * USEC_PER_MSEC);	/* 15 ms */
 	params->limit = 1000;	/* default of 1000 packets */
 	params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC);	/* 15 ms */
 	params->ecn = false;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 4/7] net: sched: pie: change initial value of pie_vars->burst_time
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests an initial value of 150 milliseconds for
the maximum time allowed for a burst of packets.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 702f75afc312..d88ab53593b3 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -92,8 +92,8 @@ static void pie_vars_init(struct pie_vars *vars)
 {
 	vars->dq_count = DQCOUNT_INVALID;
 	vars->avg_dq_rate = 0;
-	/* default of 100 ms in pschedtime */
-	vars->burst_time = PSCHED_NS2TICKS(100 * NSEC_PER_MSEC);
+	/* default of 150 ms in pschedtime */
+	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: Leslie Monis @ 2019-02-25 19:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

The current implementation scales the local alpha and beta
variables in the calculate_probability function by the same
amount for all values of drop probability below 1%.

RFC 8033 suggests using additional cases for auto-tuning
alpha and beta when the drop probability is less than 1%.

In order to add more auto-tuning cases, MAX_PROB must be
scaled by u64 instead of u32 to prevent underflow when
scaling the local alpha and beta variables in the
calculate_probability function.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/uapi/linux/pkt_sched.h |  2 +-
 net/sched/sch_pie.c            | 65 +++++++++++++++++-----------------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 0d18b1d1fbbc..1eb572ef3f27 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -954,7 +954,7 @@ enum {
 #define TCA_PIE_MAX   (__TCA_PIE_MAX - 1)
 
 struct tc_pie_xstats {
-	__u32 prob;             /* current probability */
+	__u64 prob;             /* current probability */
 	__u32 delay;            /* current delay in ms */
 	__u32 avg_dq_rate;      /* current average dq_rate in bits/pie_time */
 	__u32 packets_in;       /* total number of packets enqueued */
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d88ab53593b3..30f158582499 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -33,7 +33,7 @@
 
 #define QUEUE_THRESHOLD 16384
 #define DQCOUNT_INVALID -1
-#define MAX_PROB  0xffffffff
+#define MAX_PROB 0xffffffffffffffff
 #define PIE_SCALE 8
 
 /* parameters used */
@@ -49,7 +49,7 @@ struct pie_params {
 
 /* variables used */
 struct pie_vars {
-	u32 prob;		/* probability but scaled by u32 limit. */
+	u64 prob;		/* probability but scaled by u64 limit. */
 	psched_time_t burst_time;
 	psched_time_t qdelay;
 	psched_time_t qdelay_old;
@@ -99,8 +99,8 @@ static void pie_vars_init(struct pie_vars *vars)
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
 {
 	struct pie_sched_data *q = qdisc_priv(sch);
-	u32 rnd;
-	u32 local_prob = q->vars.prob;
+	u64 rnd;
+	u64 local_prob = q->vars.prob;
 	u32 mtu = psched_mtu(qdisc_dev(sch));
 
 	/* If there is still burst allowance left skip random early drop */
@@ -124,11 +124,11 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
 	 * probablity. Smaller packets will have lower drop prob in this case
 	 */
 	if (q->params.bytemode && packet_size <= mtu)
-		local_prob = (local_prob / mtu) * packet_size;
+		local_prob = (u64)packet_size * div_u64(local_prob, mtu);
 	else
 		local_prob = q->vars.prob;
 
-	rnd = prandom_u32();
+	prandom_bytes(&rnd, 8);
 	if (rnd < local_prob)
 		return true;
 
@@ -317,9 +317,10 @@ static void calculate_probability(struct Qdisc *sch)
 	u32 qlen = sch->qstats.backlog;	/* queue size in bytes */
 	psched_time_t qdelay = 0;	/* in pschedtime */
 	psched_time_t qdelay_old = q->vars.qdelay;	/* in pschedtime */
-	s32 delta = 0;		/* determines the change in probability */
-	u32 oldprob;
-	u32 alpha, beta;
+	s64 delta = 0;		/* determines the change in probability */
+	u64 oldprob;
+	u64 alpha, beta;
+	u32 power;
 	bool update_prob = true;
 
 	q->vars.qdelay_old = q->vars.qdelay;
@@ -339,38 +340,36 @@ static void calculate_probability(struct Qdisc *sch)
 	 * value for alpha as 0.125. In this implementation, we use values 0-32
 	 * passed from user space to represent this. Also, alpha and beta have
 	 * unit of HZ and need to be scaled before they can used to update
-	 * probability. alpha/beta are updated locally below by 1) scaling them
-	 * appropriately 2) scaling down by 16 to come to 0-2 range.
-	 * Please see paper for details.
-	 *
-	 * We scale alpha and beta differently depending on whether we are in
-	 * light, medium or high dropping mode.
+	 * probability. alpha/beta are updated locally below by scaling down
+	 * by 16 to come to 0-2 range.
 	 */
-	if (q->vars.prob < MAX_PROB / 100) {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
-	} else if (q->vars.prob < MAX_PROB / 10) {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
-	} else {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+	alpha = ((u64)q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+	beta = ((u64)q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+
+	/* We scale alpha and beta differently depending on how heavy the
+	 * congestion is. Please see RFC 8033 for details.
+	 */
+	if (q->vars.prob < MAX_PROB / 10) {
+		alpha >>= 1;
+		beta >>= 1;
+
+		power = 100;
+		while (q->vars.prob < div_u64(MAX_PROB, power) &&
+		       power <= 1000000) {
+			alpha >>= 2;
+			beta >>= 2;
+			power *= 10;
+		}
 	}
 
 	/* alpha and beta should be between 0 and 32, in multiples of 1/16 */
-	delta += alpha * ((qdelay - q->params.target));
-	delta += beta * ((qdelay - qdelay_old));
+	delta += alpha * (u64)(qdelay - q->params.target);
+	delta += beta * (u64)(qdelay - qdelay_old);
 
 	oldprob = q->vars.prob;
 
 	/* to ensure we increase probability in steps of no more than 2% */
-	if (delta > (s32)(MAX_PROB / (100 / 2)) &&
+	if (delta > (s64)(MAX_PROB / (100 / 2)) &&
 	    q->vars.prob >= MAX_PROB / 10)
 		delta = (MAX_PROB / 100) * 2;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 6/7] net: sched: pie: add derandomization mechanism
From: Leslie Monis @ 2019-02-25 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

Random dropping of packets to achieve latency control may
introduce outlier situations where packets are dropped too
close to each other or too far from each other. This can
cause the real drop percentage to temporarily deviate from
the intended drop probability. In certain scenarios, such
as a small number of simultaneous TCP flows, these
deviations can cause significant deviations in link
utilization and queuing latency.

RFC 8033 suggests using a derandomization mechanism to avoid
these deviations.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 30f158582499..916b878d3491 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -55,8 +55,10 @@ struct pie_vars {
 	psched_time_t qdelay_old;
 	u64 dq_count;		/* measured in bytes */
 	psched_time_t dq_tstamp;	/* drain rate */
+	u64 accu_prob;		/* accumulated drop probability */
 	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
 	u32 qlen_old;		/* in bytes */
+	u8 accu_prob_overflows;	/* overflows of accu_prob */
 };
 
 /* statistics gathering */
@@ -91,9 +93,11 @@ static void pie_params_init(struct pie_params *params)
 static void pie_vars_init(struct pie_vars *vars)
 {
 	vars->dq_count = DQCOUNT_INVALID;
+	vars->accu_prob = 0;
 	vars->avg_dq_rate = 0;
 	/* default of 150 ms in pschedtime */
 	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
+	vars->accu_prob_overflows = 0;
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
@@ -128,9 +132,29 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
 	else
 		local_prob = q->vars.prob;
 
+	if (local_prob == 0) {
+		q->vars.accu_prob = 0;
+		q->vars.accu_prob_overflows = 0;
+	}
+
+	if (local_prob > MAX_PROB - q->vars.accu_prob)
+		q->vars.accu_prob_overflows++;
+
+	q->vars.accu_prob += local_prob;
+
+	if (q->vars.accu_prob_overflows == 0 &&
+	    q->vars.accu_prob < (MAX_PROB / 100) * 85)
+		return false;
+	if (q->vars.accu_prob_overflows == 8 &&
+	    q->vars.accu_prob >= MAX_PROB / 2)
+		return true;
+
 	prandom_bytes(&rnd, 8);
-	if (rnd < local_prob)
+	if (rnd < local_prob) {
+		q->vars.accu_prob = 0;
+		q->vars.accu_prob_overflows = 0;
 		return true;
+	}
 
 	return false;
 }
@@ -168,6 +192,8 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 
 out:
 	q->stats.dropped++;
+	q->vars.accu_prob = 0;
+	q->vars.accu_prob_overflows = 0;
 	return qdisc_drop(skb, sch, to_free);
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 7/7] net: sched: pie: update references
From: Leslie Monis @ 2019-02-25 19:10 UTC (permalink / raw)
  To: davem
  Cc: netdev, Mohit P. Tahiliani, Dave Taht, Jamal Hadi Salim,
	Dhaval Khandla, Hrishikesh Hiraskar, Manish Kumar B,
	Sachin D . Patil, Leslie Monis
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 replaces the IETF draft for PIE

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Acked-by: Dave Taht <dave.taht@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_pie.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 916b878d3491..f8314a14a256 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,9 +17,7 @@
  * University of Oslo, Norway.
  *
  * References:
- * IETF draft submission: http://tools.ietf.org/html/draft-pan-aqm-pie-00
- * IEEE  Conference on High Performance Switching and Routing 2013 :
- * "PIE: A * Lightweight Control Scheme to Address the Bufferbloat Problem"
+ * RFC 8033: https://tools.ietf.org/html/rfc8034
  */
 
 #include <linux/module.h>
-- 
2.17.1


^ permalink raw reply related

* [PATCH][next] Bluetooth: mgmt: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-25 19:11 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S. Miller
  Cc: linux-bluetooth, netdev, linux-kernel, Gustavo A. R. Silva

Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes, in particular in the
context in which this code is being used.

So, change the following form:

sizeof(*rp) + (sizeof(rp->entry[0]) * count);

to :

struct_size(rp, entry, count)

Notice that, in this case, variable rp_len is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 net/bluetooth/mgmt.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ccce954f8146..1e2acaddcdfd 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -474,7 +474,6 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
 {
 	struct mgmt_rp_read_ext_index_list *rp;
 	struct hci_dev *d;
-	size_t rp_len;
 	u16 count;
 	int err;
 
@@ -488,8 +487,7 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
 			count++;
 	}
 
-	rp_len = sizeof(*rp) + (sizeof(rp->entry[0]) * count);
-	rp = kmalloc(rp_len, GFP_ATOMIC);
+	rp = kmalloc(struct_size(rp, entry, count), GFP_ATOMIC);
 	if (!rp) {
 		read_unlock(&hci_dev_list_lock);
 		return -ENOMEM;
@@ -525,7 +523,6 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
 	}
 
 	rp->num_controllers = cpu_to_le16(count);
-	rp_len = sizeof(*rp) + (sizeof(rp->entry[0]) * count);
 
 	read_unlock(&hci_dev_list_lock);
 
@@ -538,7 +535,8 @@ static int read_ext_index_list(struct sock *sk, struct hci_dev *hdev,
 	hci_sock_clear_flag(sk, HCI_MGMT_UNCONF_INDEX_EVENTS);
 
 	err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
-				MGMT_OP_READ_EXT_INDEX_LIST, 0, rp, rp_len);
+				MGMT_OP_READ_EXT_INDEX_LIST, 0, rp,
+				struct_size(rp, entry, count));
 
 	kfree(rp);
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: Ido Schimmel @ 2019-02-25 19:17 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190225190132.GA23478@embeddedor>

On Mon, Feb 25, 2019 at 01:01:32PM -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     struct boo entry[];
> };
> 
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = kzalloc(size, GFP_KERNEL)
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
> 
> Notice that, in this case, variable alloc_size is not necessary, hence
> it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

Thanks

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: phy: aquantia: add hwmon support
From: Florian Fainelli @ 2019-02-25 19:19 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <47178fa9-1065-87f8-bcfa-59d8764d21f8@gmail.com>

On 2/25/19 10:56 AM, Heiner Kallweit wrote:
> This adds HWMON support for the temperature sensor and the related
> alarms on the 107/108/109 chips. This patch is based on work from
> Nikita and Andrew. I added:
> - support for changing alarm thresholds via sysfs
> - move HWMON code to a separate source file to improve maintainability
> - smaller changes like using IS_REACHABLE instead of ifdef
>   (avoids problems if PHY driver is built in and HWMON is a module)
> 
> v2:
> - remove struct aqr_priv
> - rename header file to aquantia.h
> v3:
> - add conditional compiling of aquantia_hwmon.c
> - improve converting sensor register values to/from long
> - add helper aqr_hwmon_test_bit
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* RE: [net-next 05/14] i40e: fix up 32 bit timespec references
From: Keller, Jacob E @ 2019-02-25 19:20 UTC (permalink / raw)
  To: Arnd Bergmann, Kirsher, Jeffrey T
  Cc: David Miller, Brandeburg, Jesse, Networking, nhorman@redhat.com,
	sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <CAK8P3a3fSofgDdLZ3S+MaEVz8DHnX6HO6xGiRWfCjQ614Kf4Cg@mail.gmail.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Arnd Bergmann
> Sent: Monday, February 25, 2019 6:46 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: David Miller <davem@davemloft.net>; Brandeburg, Jesse
> <jesse.brandeburg@intel.com>; Networking <netdev@vger.kernel.org>;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 05/14] i40e: fix up 32 bit timespec references
> 
> On Wed, Jul 26, 2017 at 12:33 PM Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> >
> > From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >
> > As it turns out there was only a small set of errors
> > on 32 bit, and we just needed to be using the right calls
> > for dealing with timespec64 variables.
> 
> I just stumbled over code added by this older patch, and can't make sense
> of the commit description here. Was this an attempt to fix a bug, or
> just a cleanup?
> 
> >
> > -       then = ns_to_timespec64(delta);
> >         mutex_lock(&pf->tmreg_lock);
> >
> >         i40e_ptp_read(pf, &now);
> > -       now = timespec64_add(now, then);
> > +       timespec64_add_ns(&now, delta);
> >         i40e_ptp_write(pf, (const struct timespec64 *)&now);
> 
> The problem I noticed here is that 'delta' is a user provided 64-bit
> number from clock_adjtime(), and timespec64_add_ns() performs uses
> a repeated addition instead of a div/mod pair. When the number
> is large, we may end up adding a single second 8 billion times,
> which may take a while even on a fast CPU.
> 

It looked like the timespec64_add_ns does a div/mod pair...? Or am I mis-reading how the function is implemented? Quite probably.

Either way, the code is incorrect, because timespec64_add_ns doesn't actually work with signed values. A negative delta actually ends up resulting in a significant positive addition. Woops!

> Should the commit 0ac30ce43323 ("i40e: fix up 32 bit timespec
> references") just be reverted?
> 

Yea, let's revert it.

Thanks,
Jake

>       Arnd



^ permalink raw reply

* Re: [PATCH v2 bpf-next 4/4] tools/bpftool: recognize bpf_prog_info runtime and runcnt
From: Andrii Nakryiko @ 2019-02-25 19:24 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, davem, edumazet, netdev, bpf, Kernel Team
In-Reply-To: <cf8b7ffc-22f9-a095-7614-2d191ba53522@iogearbox.net>

On Mon, Feb 25, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/23/2019 06:44 PM, Alexei Starovoitov wrote:
> > $ bpftool p s
> > 1: kprobe  tag a56587d488d216c9  gpl runtime 79786 runcnt 8
> >       loaded_at 2019-02-22T12:22:51-0800  uid 0
> >       xlated 352B  not jited  memlock 4096B
> >
> > $ bpftool --json --pretty p s
> > [{
> >         "id": 1,
> >         "type": "kprobe",
> >         "tag": "a56587d488d216c9",
> >         "gpl_compatible": true,
> >         "run_time_ns": 79786,
> >         "run_cnt": 8,
> >         "loaded_at": 1550866971,
> >         "uid": 0,
> >         "bytes_xlated": 352,
> >         "jited": false,
> >         "bytes_memlock": 4096
> >     }
> > ]
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
> >  tools/bpf/bpftool/prog.c                         | 7 +++++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index 12bc1e2d4b46..d8b46a8f17e0 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > @@ -171,7 +171,7 @@ EXAMPLES
> >
> >  ::
> >
> > -    10: xdp  name some_prog  tag 005a3d2123620c8b  gpl
> > +    10: xdp  name some_prog  tag 005a3d2123620c8b  gpl runtime 81632 runcnt 10
> >              loaded_at 2017-09-29T20:11:00+0000  uid 0
> >              xlated 528B  jited 370B  memlock 4096B  map_ids 10
> >
> > @@ -184,6 +184,8 @@ EXAMPLES
> >              "type": "xdp",
> >              "tag": "005a3d2123620c8b",
> >              "gpl_compatible": true,
> > +            "run_time_ns": 81632,
> > +            "run_cnt": 10,
> >              "loaded_at": 1506715860,
> >              "uid": 0,
> >              "bytes_xlated": 528,
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index db978c8d76a8..cdb6bd424340 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
> >                    info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
> >
> >       jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
> > +     if (info->runtime) {
> > +             jsonw_uint_field(json_wtr, "run_time_ns", info->runtime);
> > +             jsonw_uint_field(json_wtr, "run_cnt", info->runcnt);
>
> Didn't Andrii mean to generally name these fields run_time_ns and run_cnt, so also
> uapi aside from just changing bpftool output (why keeping these names not the same)?

Yeah, I think consistency is important. This also applies to patch #1
(bpf: enable program stats), which exposes same stats through procfs.
Ideally all those names would be consistent (for obvious reasons). As
for _ns, I realize it's a bit longer and more verbose, but having time
units as part of a name is really great for users and doesn't require
extra documentation (and extra man lookups).


>
> > +     }
> >
> >       print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
> >
> > @@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> >       fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> >       print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> >       printf("%s", info->gpl_compatible ? "  gpl" : "");
> > +     if (info->runtime)
> > +             printf(" runtime %lld runcnt %lld",
> > +                    info->runtime, info->runcnt);
> >       printf("\n");
> >
> >       if (info->load_time) {
> >
>

^ permalink raw reply

* Re: INFO: task hung in rtnetlink_rcv_msg
From: Jesper Dangaard Brouer @ 2019-02-25 19:26 UTC (permalink / raw)
  To: Joe Perches
  Cc: Daniel Borkmann, Jakub Kicinski, syzbot, ast, christian, davem,
	dsahern, hawk, idosch, john.fastabend, kafai, ktkhai,
	linux-kernel, netdev, petrm, roopa, songliubraving,
	syzkaller-bugs, xdp-newbies, yhs, brouer
In-Reply-To: <57807dca3bf554d0a07356ba1e53fae4d58cfb51.camel@perches.com>

On Mon, 25 Feb 2019 08:55:40 -0800
Joe Perches <joe@perches.com> wrote:

> On Mon, 2019-02-25 at 09:39 +0100, Jesper Dangaard Brouer wrote:
> > On Sat, 23 Feb 2019 01:47:00 +0100
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >   
> > > On 02/22/2019 10:45 PM, Jakub Kicinski wrote:  
> > > > On Fri, 22 Feb 2019 12:14:57 -0800, Joe Perches wrote:    
> > > > > On Fri, 2019-02-22 at 12:01 -0800, Jakub Kicinski wrote:    
> > > > > > Hi!
> > > > > > 
> > > > > > Seems like something funny is going on with get_maintainer.pl since XDP
> > > > > > entry got added.  We seem to have been CCed on:      
> > > > > 
> > > > > I suggest removing the MAINTAINERS line with
> > > > > 
> > > > > 	K:	xdp
> > > > > 
> > > > > as xdp is a pretty generic file/patch content
> > > > > regex match for the K: type
> > > > > 
> > > > > $ git grep --name-only xdp | wc -l
> > > > > 236  
> > 
> > I'm unsure how K: works, but you grep definitely selects some wrong files.
> > 
> > I tried with "xdp_":
> >  git grep --name-only xdp_
> > 
> > That does catch all the driver that have XDP support, which is the
> > point of the exercise (to catch drivers).
> > 
> > It does contain a couple of false-positives:
> >   drivers/net/ethernet/neterion/vxge/vxge-traffic.c
> >   drivers/thunderbolt/tb_msgs.h
> >   drivers/thunderbolt/xdomain.c
> >   sound/soc/codecs/rt5670.c
> > 
> > Via the pattern '[^a-z]xdp_' I'm only left with the thunderbolt
> > false-positive, as it have a data struct's called tb_xdp_*.  
> 
> Then it's probably better to use
> 
> K:	\Wxdp_
> 
> and if XDP_ is also to be matched, maybe
> 
> K:	(?i:\Wxdp_)
> 

The last one seems to be the best at matching the relevant files :-)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Florian Fainelli @ 2019-02-25 19:47 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev@vger.kernel.org, David S. Miller, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
	Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190225094940.GA15128@splinter>

On 2/25/19 1:49 AM, Ido Schimmel wrote:
> On Sun, Feb 24, 2019 at 08:47:27AM -0800, Florian Fainelli wrote:
>> Le 2/23/19 à 2:32 AM, Ido Schimmel a écrit :
>>> On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:
>>>> -	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
>>>> +	if (attr & SWITCHDEV_F_DEFER)
>>>> +		rc = call_switchdev_blocking_notifiers(nt, dev,
>>>> +						       &attr_info.info, NULL);
>>>> +	else
>>>> +		rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
>>>
>>> I don't believe this is needed. You're calling this function from
>>> switchdev_port_attr_set_now() which is always called from process
>>> context. switchdev_port_attr_set() takes care of that. Similar to
>>> switchdev_port_obj_add().
>>
>> Except for net/bridge/br_switchdev.c when we check the bridge port's
>> flags support with PRE_BRIDGE_FLAGS. In that case we are executing from
>> the caller (atomic) context and we can't defer otherwise that trumps the
>> whole idea of being able to do a quick check and return that to the
>> caller that we cannot support specific flags. How would you recommend
>> approaching that?
> 
> In this case you can invoke call_switchdev_notifiers() directly from
> br_switchdev_set_port_flag(). Eventually switchdev_port_attr_set() will
> be gone and bridge code will invoke the notifiers directly.

That can be done, but it still requires the target driver (mlxsw,
ocelot, dsa, etc.) to support attribute notification from blocking and
non-blocking context. Are you fine with that?
-- 
Florian

^ permalink raw reply

* Re: [PATCH v2 bpf-next 2/4] bpf: expose program stats via bpf_prog_info
From: Andrii Nakryiko @ 2019-02-25 19:47 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, Daniel Borkmann, edumazet, netdev, bpf, Kernel Team
In-Reply-To: <20190223174422.663270-3-ast@kernel.org>

On Sat, Feb 23, 2019 at 9:44 AM Alexei Starovoitov <ast@kernel.org> wrote:
>
> Return bpf program runtime and runcnt via bpf_prog_info
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  include/uapi/linux/bpf.h | 2 ++
>  kernel/bpf/syscall.c     | 5 +++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..d2cb85d85b39 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2813,6 +2813,8 @@ struct bpf_prog_info {
>         __u32 jited_line_info_rec_size;
>         __u32 nr_prog_tags;
>         __aligned_u64 prog_tags;
> +       __u64 runtime;
> +       __u64 runcnt;
>  } __attribute__((aligned(8)));
>
>  struct bpf_map_info {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 9a8c990c5df4..3405e0a4cf77 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2152,6 +2152,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
>         struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info);
>         struct bpf_prog_info info = {};
>         u32 info_len = attr->info.info_len;
> +       struct bpf_prog_stats stats;
>         char __user *uinsns;
>         u32 ulen;
>         int err;
> @@ -2191,6 +2192,10 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
>         if (err)
>                 return err;
>
> +       bpf_prog_get_stats(prog, &stats);
> +       info.runtime = stats.nsecs;
> +       info.runcnt = stats.cnt;
> +

lgtm, but see naming discussion in other thread.

Acked-by: Andrii Nakryiko <andriin@fb.com>

>         if (!capable(CAP_SYS_ADMIN)) {
>                 info.jited_prog_len = 0;
>                 info.xlated_prog_len = 0;
> --
> 2.20.0
>

^ permalink raw reply

* Re: [PATCH 1/2] bpf: context casting for tail call
From: kbuild test robot @ 2019-02-25 20:10 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: kbuild-all, netdev
In-Reply-To: <201902251554.x1PFsWmr017326@userv0122.oracle.com>

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

Hi Kris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]
[also build test ERROR on v5.0-rc8 next-20190225]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kris-Van-Hees/bpf-context-casting-for-tail-call-and-gtrace-prog-type/20190226-034827
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-x016-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   kernel/bpf/core.c: In function 'bpf_prog_array_can_cast':
>> kernel/bpf/core.c:1657:8: error: 'bpf_prog_types' undeclared (first use in this function); did you mean 'bpf_prog_type'?
     ops = bpf_prog_types[array->owner_prog_type];
           ^~~~~~~~~~~~~~
           bpf_prog_type
   kernel/bpf/core.c:1657:8: note: each undeclared identifier is reported only once for each function it appears in

vim +1657 kernel/bpf/core.c

  1649	
  1650	bool bpf_prog_array_can_cast(struct bpf_array *array, const struct bpf_prog *fp)
  1651	{
  1652		const struct bpf_prog_ops *ops;
  1653	
  1654		if (array->owner_jited != fp->jited)
  1655			return false;
  1656	
> 1657		ops = bpf_prog_types[array->owner_prog_type];
  1658		if (ops->can_cast)
  1659			return ops->can_cast(fp->type, array->owner_prog_type);
  1660	
  1661		return false;
  1662	}
  1663	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28326 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] bpf: context casting for tail call
From: kbuild test robot @ 2019-02-25 20:10 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: kbuild-all, netdev
In-Reply-To: <201902251554.x1PFsWmr017326@userv0122.oracle.com>

[-- Attachment #1: Type: text/plain, Size: 8559 bytes --]

Hi Kris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]
[also build test WARNING on v5.0-rc8 next-20190225]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kris-Van-Hees/bpf-context-casting-for-tail-call-and-gtrace-prog-type/20190226-034827
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   kernel//bpf/core.c: In function '___bpf_prog_run':
>> kernel//bpf/core.c:1452:13: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
       BPF_R1 = (u64) ctx;
                ^

vim +1452 kernel//bpf/core.c

  1290	
  1291	select_insn:
  1292		goto *jumptable[insn->code];
  1293	
  1294		/* ALU */
  1295	#define ALU(OPCODE, OP)			\
  1296		ALU64_##OPCODE##_X:		\
  1297			DST = DST OP SRC;	\
  1298			CONT;			\
  1299		ALU_##OPCODE##_X:		\
  1300			DST = (u32) DST OP (u32) SRC;	\
  1301			CONT;			\
  1302		ALU64_##OPCODE##_K:		\
  1303			DST = DST OP IMM;		\
  1304			CONT;			\
  1305		ALU_##OPCODE##_K:		\
  1306			DST = (u32) DST OP (u32) IMM;	\
  1307			CONT;
  1308	
  1309		ALU(ADD,  +)
  1310		ALU(SUB,  -)
  1311		ALU(AND,  &)
  1312		ALU(OR,   |)
  1313		ALU(LSH, <<)
  1314		ALU(RSH, >>)
  1315		ALU(XOR,  ^)
  1316		ALU(MUL,  *)
  1317	#undef ALU
  1318		ALU_NEG:
  1319			DST = (u32) -DST;
  1320			CONT;
  1321		ALU64_NEG:
  1322			DST = -DST;
  1323			CONT;
  1324		ALU_MOV_X:
  1325			DST = (u32) SRC;
  1326			CONT;
  1327		ALU_MOV_K:
  1328			DST = (u32) IMM;
  1329			CONT;
  1330		ALU64_MOV_X:
  1331			DST = SRC;
  1332			CONT;
  1333		ALU64_MOV_K:
  1334			DST = IMM;
  1335			CONT;
  1336		LD_IMM_DW:
  1337			DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
  1338			insn++;
  1339			CONT;
  1340		ALU_ARSH_X:
  1341			DST = (u64) (u32) ((*(s32 *) &DST) >> SRC);
  1342			CONT;
  1343		ALU_ARSH_K:
  1344			DST = (u64) (u32) ((*(s32 *) &DST) >> IMM);
  1345			CONT;
  1346		ALU64_ARSH_X:
  1347			(*(s64 *) &DST) >>= SRC;
  1348			CONT;
  1349		ALU64_ARSH_K:
  1350			(*(s64 *) &DST) >>= IMM;
  1351			CONT;
  1352		ALU64_MOD_X:
  1353			div64_u64_rem(DST, SRC, &AX);
  1354			DST = AX;
  1355			CONT;
  1356		ALU_MOD_X:
  1357			AX = (u32) DST;
  1358			DST = do_div(AX, (u32) SRC);
  1359			CONT;
  1360		ALU64_MOD_K:
  1361			div64_u64_rem(DST, IMM, &AX);
  1362			DST = AX;
  1363			CONT;
  1364		ALU_MOD_K:
  1365			AX = (u32) DST;
  1366			DST = do_div(AX, (u32) IMM);
  1367			CONT;
  1368		ALU64_DIV_X:
  1369			DST = div64_u64(DST, SRC);
  1370			CONT;
  1371		ALU_DIV_X:
  1372			AX = (u32) DST;
  1373			do_div(AX, (u32) SRC);
  1374			DST = (u32) AX;
  1375			CONT;
  1376		ALU64_DIV_K:
  1377			DST = div64_u64(DST, IMM);
  1378			CONT;
  1379		ALU_DIV_K:
  1380			AX = (u32) DST;
  1381			do_div(AX, (u32) IMM);
  1382			DST = (u32) AX;
  1383			CONT;
  1384		ALU_END_TO_BE:
  1385			switch (IMM) {
  1386			case 16:
  1387				DST = (__force u16) cpu_to_be16(DST);
  1388				break;
  1389			case 32:
  1390				DST = (__force u32) cpu_to_be32(DST);
  1391				break;
  1392			case 64:
  1393				DST = (__force u64) cpu_to_be64(DST);
  1394				break;
  1395			}
  1396			CONT;
  1397		ALU_END_TO_LE:
  1398			switch (IMM) {
  1399			case 16:
  1400				DST = (__force u16) cpu_to_le16(DST);
  1401				break;
  1402			case 32:
  1403				DST = (__force u32) cpu_to_le32(DST);
  1404				break;
  1405			case 64:
  1406				DST = (__force u64) cpu_to_le64(DST);
  1407				break;
  1408			}
  1409			CONT;
  1410	
  1411		/* CALL */
  1412		JMP_CALL:
  1413			/* Function call scratches BPF_R1-BPF_R5 registers,
  1414			 * preserves BPF_R6-BPF_R9, and stores return value
  1415			 * into BPF_R0.
  1416			 */
  1417			BPF_R0 = (__bpf_call_base + insn->imm)(BPF_R1, BPF_R2, BPF_R3,
  1418							       BPF_R4, BPF_R5);
  1419			CONT;
  1420	
  1421		JMP_CALL_ARGS:
  1422			BPF_R0 = (__bpf_call_base_args + insn->imm)(BPF_R1, BPF_R2,
  1423								    BPF_R3, BPF_R4,
  1424								    BPF_R5,
  1425								    insn + insn->off + 1);
  1426			CONT;
  1427	
  1428		JMP_TAIL_CALL: {
  1429			void *ctx = (void *) (unsigned long) BPF_R1;
  1430			struct bpf_map *map = (struct bpf_map *) (unsigned long) BPF_R2;
  1431			struct bpf_array *array = container_of(map, struct bpf_array, map);
  1432			struct bpf_prog *prog;
  1433			u32 index = BPF_R3;
  1434			u32 type = BPF_R4;
  1435	
  1436			if (unlikely(index >= array->map.max_entries))
  1437				goto out;
  1438			if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
  1439				goto out;
  1440	
  1441			tail_call_cnt++;
  1442	
  1443			prog = READ_ONCE(array->ptrs[index]);
  1444			if (!prog)
  1445				goto out;
  1446			if (prog->aux->ops->cast_context) {
  1447				ctx = prog->aux->ops->cast_context(type, prog->type,
  1448								   ctx);
  1449				if (!ctx)
  1450					goto out;
  1451	
> 1452				BPF_R1 = (u64) ctx;
  1453			}
  1454	
  1455			/* ARG1 at this point is guaranteed to point to CTX from
  1456			 * the verifier side due to the fact that the tail call is
  1457			 * handeled like a helper, that is, bpf_tail_call_proto,
  1458			 * where arg1_type is ARG_PTR_TO_CTX.
  1459			 */
  1460			insn = prog->insnsi;
  1461			goto select_insn;
  1462	out:
  1463			CONT;
  1464		}
  1465		JMP_JA:
  1466			insn += insn->off;
  1467			CONT;
  1468		JMP_EXIT:
  1469			return BPF_R0;
  1470		/* JMP */
  1471	#define COND_JMP(SIGN, OPCODE, CMP_OP)				\
  1472		JMP_##OPCODE##_X:					\
  1473			if ((SIGN##64) DST CMP_OP (SIGN##64) SRC) {	\
  1474				insn += insn->off;			\
  1475				CONT_JMP;				\
  1476			}						\
  1477			CONT;						\
  1478		JMP32_##OPCODE##_X:					\
  1479			if ((SIGN##32) DST CMP_OP (SIGN##32) SRC) {	\
  1480				insn += insn->off;			\
  1481				CONT_JMP;				\
  1482			}						\
  1483			CONT;						\
  1484		JMP_##OPCODE##_K:					\
  1485			if ((SIGN##64) DST CMP_OP (SIGN##64) IMM) {	\
  1486				insn += insn->off;			\
  1487				CONT_JMP;				\
  1488			}						\
  1489			CONT;						\
  1490		JMP32_##OPCODE##_K:					\
  1491			if ((SIGN##32) DST CMP_OP (SIGN##32) IMM) {	\
  1492				insn += insn->off;			\
  1493				CONT_JMP;				\
  1494			}						\
  1495			CONT;
  1496		COND_JMP(u, JEQ, ==)
  1497		COND_JMP(u, JNE, !=)
  1498		COND_JMP(u, JGT, >)
  1499		COND_JMP(u, JLT, <)
  1500		COND_JMP(u, JGE, >=)
  1501		COND_JMP(u, JLE, <=)
  1502		COND_JMP(u, JSET, &)
  1503		COND_JMP(s, JSGT, >)
  1504		COND_JMP(s, JSLT, <)
  1505		COND_JMP(s, JSGE, >=)
  1506		COND_JMP(s, JSLE, <=)
  1507	#undef COND_JMP
  1508		/* STX and ST and LDX*/
  1509	#define LDST(SIZEOP, SIZE)						\
  1510		STX_MEM_##SIZEOP:						\
  1511			*(SIZE *)(unsigned long) (DST + insn->off) = SRC;	\
  1512			CONT;							\
  1513		ST_MEM_##SIZEOP:						\
  1514			*(SIZE *)(unsigned long) (DST + insn->off) = IMM;	\
  1515			CONT;							\
  1516		LDX_MEM_##SIZEOP:						\
  1517			DST = *(SIZE *)(unsigned long) (SRC + insn->off);	\
  1518			CONT;
  1519	
  1520		LDST(B,   u8)
  1521		LDST(H,  u16)
  1522		LDST(W,  u32)
  1523		LDST(DW, u64)
  1524	#undef LDST
  1525		STX_XADD_W: /* lock xadd *(u32 *)(dst_reg + off16) += src_reg */
  1526			atomic_add((u32) SRC, (atomic_t *)(unsigned long)
  1527				   (DST + insn->off));
  1528			CONT;
  1529		STX_XADD_DW: /* lock xadd *(u64 *)(dst_reg + off16) += src_reg */
  1530			atomic64_add((u64) SRC, (atomic64_t *)(unsigned long)
  1531				     (DST + insn->off));
  1532			CONT;
  1533	
  1534		default_label:
  1535			/* If we ever reach this, we have a bug somewhere. Die hard here
  1536			 * instead of just returning 0; we could be somewhere in a subprog,
  1537			 * so execution could continue otherwise which we do /not/ want.
  1538			 *
  1539			 * Note, verifier whitelists all opcodes in bpf_opcode_in_insntable().
  1540			 */
  1541			pr_warn("BPF interpreter: unknown opcode %02x\n", insn->code);
  1542			BUG_ON(1);
  1543			return 0;
  1544	}
  1545	STACK_FRAME_NON_STANDARD(___bpf_prog_run); /* jump table */
  1546	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56355 bytes --]

^ permalink raw reply


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