Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Alexei Starovoitov @ 2019-01-24 23:58 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexei Starovoitov, davem, daniel, jakub.kicinski, netdev,
	kernel-team, mingo, will.deacon, Paul McKenney, jannh
In-Reply-To: <20190124180109.GA27771@hirez.programming.kicks-ass.net>

On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> 
> Thanks for having kernel/locking people on Cc...
> 
> On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:
> 
> > Implementation details:
> > - on !SMP bpf_spin_lock() becomes nop
> 
> Because no BPF program is preemptible? I don't see any assertions or
> even a comment that says this code is non-preemptible.
> 
> AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
> which is not sufficient.

nope. all bpf prog types disable preemption. That is must have for all
sorts of things to work properly.
If there is a prog type that doing rcu_read_lock only it's a serious bug.
About a year or so ago we audited everything specifically to make
sure everything disables preemption before calling bpf progs.
I'm pretty sure nothing crept in in the mean time.

> > - on architectures that don't support queued_spin_lock trivial lock is used.
> >   Note that arch_spin_lock cannot be used, since not all archs agree that
> >   zero == unlocked and sizeof(arch_spinlock_t) != sizeof(__u32).
> 
> I really don't much like direct usage of qspinlock; esp. not as a
> surprise.
> 
> Why does it matter if 0 means unlocked; that's what
> __ARCH_SPIN_LOCK_UNLOCKED is for.
> 
> I get the sizeof(__u32) thing, but why not key off of that?

what do you mean by 'key off of that' ?
to use arch_spinlock_t instead of qspinlock ?
That was my first attempt, but then I painfully found that
its size on parisc is 16 bytes and we're not going to penalize bpf
to waste that much space because of single architecture.
sizeof(arch_spinlock_t) can be 1 byte too (on sparc).
That would fit in __u32, but I figured it's cleaner to use qspinlock
on all archs that support it and dumb_spin_lock on archs that dont.

Another option is use to arch_spinlock_t when its sizeof==4
and use dumb_spin_lock otherwise.
It's doable, but imo still less clean than using qspinlock
due to zero init. Since zero init is a lot less map work
that zero inits all elements already.

If arch_spinlock_t is used than at map init time we would need to
walk all elements and do __ARCH_SPIN_LOCK_UNLOCKED assignment
(and maps can have millions of elements).
Not horrible, but 100% waste of cycles for x86/arm64 where qspinlock
is used. Such waste can be workaround further by doing ugly
#idef __ARCH_SPIN_LOCK_UNLOCKED == 0 -> don't do init loop.
And then add another #ifdef for archs with sizeof(arch_spinlock_t)!=4
to keep zero init for all map types that support bpf_spin_lock
via dumb_spin_lock.
Clearly at that point we're getting into ugliness everywhere.
Hence I've used qspinlock directly.

> > Next steps:
> > - allow bpf_spin_lock in other map types (like cgroup local storage)
> > - introduce BPF_F_LOCK flag for bpf_map_update() syscall and helper
> >   to request kernel to grab bpf_spin_lock before rewriting the value.
> >   That will serialize access to map elements.
> 
> So clearly this map stuff is shared between bpf proglets, otherwise
> there would not be a need for locking. But what happens if one is from
> task context and another from IRQ context?
> 
> I don't see a local_irq_save()/restore() anywhere. What avoids the
> trivial lock inversion?

> and from NMI ...

progs are not preemptable and map syscall accessors have bpf_prog_active counters.
So nmi/kprobe progs will not be running when syscall is running.
Hence dead lock is not possible and irq_save is not needed.

> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a74972b07e74..2e98e4caf5aa 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -221,6 +221,63 @@ const struct bpf_func_proto bpf_get_current_comm_proto = {
> >  	.arg2_type	= ARG_CONST_SIZE,
> >  };
> >  
> > +#ifndef CONFIG_QUEUED_SPINLOCKS
> > +struct dumb_spin_lock {
> > +	atomic_t val;
> > +};
> > +#endif
> > +
> > +notrace BPF_CALL_1(bpf_spin_lock, struct bpf_spin_lock *, lock)
> > +{
> > +#if defined(CONFIG_SMP)
> > +#ifdef CONFIG_QUEUED_SPINLOCKS
> > +	struct qspinlock *qlock = (void *)lock;
> > +
> > +	BUILD_BUG_ON(sizeof(*qlock) != sizeof(*lock));
> > +	queued_spin_lock(qlock);
> > +#else
> > +	struct dumb_spin_lock *qlock = (void *)lock;
> > +
> > +	BUILD_BUG_ON(sizeof(*qlock) != sizeof(*lock));
> > +	do {
> > +		while (atomic_read(&qlock->val) != 0)
> > +			cpu_relax();
> > +	} while (atomic_cmpxchg(&qlock->val, 0, 1) != 0);
> > +#endif
> > +#endif
> > +	return 0;
> > +}
> > +
> > +const struct bpf_func_proto bpf_spin_lock_proto = {
> > +	.func		= bpf_spin_lock,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_VOID,
> > +	.arg1_type	= ARG_PTR_TO_SPIN_LOCK,
> > +};
> > +
> > +notrace BPF_CALL_1(bpf_spin_unlock, struct bpf_spin_lock *, lock)
> > +{
> > +#if defined(CONFIG_SMP)
> > +#ifdef CONFIG_QUEUED_SPINLOCKS
> > +	struct qspinlock *qlock = (void *)lock;
> > +
> > +	queued_spin_unlock(qlock);
> > +#else
> > +	struct dumb_spin_lock *qlock = (void *)lock;
> > +
> > +	atomic_set(&qlock->val, 0);
> 
> And this is broken... That should've been atomic_set_release() at the
> very least.

right. good catch.

> And this would again be the moment where I go pester you about the BPF
> memory model :-)

hehe :)
How do you propose to define it in a way that it applies to all archs
and yet doesn't penalize x86 ?
"Assume minimum execution ordering model" the way kernel does
unfortunately is not usable, since bpf doesn't have a luxury
of using nice #defines that convert into nops on x86.


^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Paul E. McKenney @ 2019-01-24 23:42 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexei Starovoitov, davem, daniel, jakub.kicinski, netdev,
	kernel-team, mingo, will.deacon, jannh
In-Reply-To: <20190124185652.GB17767@hirez.programming.kicks-ass.net>

On Thu, Jan 24, 2019 at 07:56:52PM +0100, Peter Zijlstra wrote:
> On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> > 
> > Thanks for having kernel/locking people on Cc...
> > 
> > On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:
> > 
> > > Implementation details:
> > > - on !SMP bpf_spin_lock() becomes nop
> > 
> > Because no BPF program is preemptible? I don't see any assertions or
> > even a comment that says this code is non-preemptible.
> > 
> > AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
> > which is not sufficient.
> > 
> > > - on architectures that don't support queued_spin_lock trivial lock is used.
> > >   Note that arch_spin_lock cannot be used, since not all archs agree that
> > >   zero == unlocked and sizeof(arch_spinlock_t) != sizeof(__u32).
> > 
> > I really don't much like direct usage of qspinlock; esp. not as a
> > surprise.

Substituting the lightweight-reader SRCU as discussed earlier would allow
use of a more generic locking primitive, for example, one that allowed
blocking, at least in cases were the context allowed this.

git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
branch srcu-lr.2019.01.16a.

One advantage of a more generic locking primitive would be keeping BPF
programs independent of internal changes to spinlock primitives.

							Thanx, Paul

> > Why does it matter if 0 means unlocked; that's what
> > __ARCH_SPIN_LOCK_UNLOCKED is for.
> > 
> > I get the sizeof(__u32) thing, but why not key off of that?
> > 
> > > Next steps:
> > > - allow bpf_spin_lock in other map types (like cgroup local storage)
> > > - introduce BPF_F_LOCK flag for bpf_map_update() syscall and helper
> > >   to request kernel to grab bpf_spin_lock before rewriting the value.
> > >   That will serialize access to map elements.
> > 
> > So clearly this map stuff is shared between bpf proglets, otherwise
> > there would not be a need for locking. But what happens if one is from
> > task context and another from IRQ context?
> > 
> > I don't see a local_irq_save()/restore() anywhere. What avoids the
> > trivial lock inversion?
> 
> Also; what about BPF running from NMI context and using locks?
> 


^ permalink raw reply

* [PATCH v6 3/4] Bluetooth: Allow driver specific cmd timeout handling
From: Rajat Jain @ 2019-01-24 23:28 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
	David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
	linux-bluetooth, linux-kernel, linux-usb, netdev
  Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
	sukumar.ghorai
In-Reply-To: <20190124232814.252661-1-rajatja@google.com>

Add a hook to allow the BT driver to do device or command specific
handling in case of timeouts. This is to be used by Intel driver to
reset the device after certain number of timeouts.

Signed-off-by: Rajat Jain <rajatja@google.com>
---
v6: Dropped the "sent command" parameter from cmd_timeout()
v5: Drop the quirk, and rename the hook function to cmd_timeout()
v4: same as v1
v3: same as v1
v2: same as v1

 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/hci_core.c         | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e5ea633ea368..094e61e07030 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -437,6 +437,7 @@ struct hci_dev {
 	int (*post_init)(struct hci_dev *hdev);
 	int (*set_diag)(struct hci_dev *hdev, bool enable);
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
+	void (*cmd_timeout)(struct hci_dev *hdev);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7352fe85674b..75793265ba9e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2578,6 +2578,9 @@ static void hci_cmd_timeout(struct work_struct *work)
 		bt_dev_err(hdev, "command tx timeout");
 	}
 
+	if (hdev->cmd_timeout)
+		hdev->cmd_timeout(hdev);
+
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
 }
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* [PATCH v6 4/4] Bluetooth: btusb: Use the cmd_timeout method to reset the Intel BT chip
From: Rajat Jain @ 2019-01-24 23:28 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
	David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
	linux-bluetooth, linux-kernel, linux-usb, netdev
  Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
	sukumar.ghorai
In-Reply-To: <20190124232814.252661-1-rajatja@google.com>

If the platform provides it, use the reset gpio to reset the Intel BT
chip, as part of cmd_timeout handling. This has been found helpful on
Intel bluetooth controllers where the firmware gets stuck and the only
way out is a hard reset pin provided by the platform.

Signed-off-by: Rajat Jain <rajatja@google.com>
---
v6: Move the cmd_timeout() hook assignment with other hooks, move the
    reset_gpio check in the timeout function.
v5: Rename the hook to cmd_timeout, and wait for 5 timeouts before
    resetting the device.
v4: Use data->flags instead of clearing the quirk in btusb_hw_reset()
v3: Better error handling for gpiod_get_optional()
v2: Handle the EPROBE_DEFER case.

 drivers/bluetooth/btusb.c | 54 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 4761499db9ee..5de0c2e59b97 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -29,6 +29,7 @@
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/suspend.h>
+#include <linux/gpio/consumer.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -439,6 +440,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = {
 #define BTUSB_BOOTING		9
 #define BTUSB_DIAG_RUNNING	10
 #define BTUSB_OOB_WAKE_ENABLED	11
+#define BTUSB_HW_RESET_ACTIVE	12
 
 struct btusb_data {
 	struct hci_dev       *hdev;
@@ -476,6 +478,8 @@ struct btusb_data {
 	struct usb_endpoint_descriptor *diag_tx_ep;
 	struct usb_endpoint_descriptor *diag_rx_ep;
 
+	struct gpio_desc *reset_gpio;
+
 	__u8 cmdreq_type;
 	__u8 cmdreq;
 
@@ -489,8 +493,41 @@ struct btusb_data {
 	int (*setup_on_usb)(struct hci_dev *hdev);
 
 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
+	unsigned cmd_timeout_cnt;
 };
 
+
+static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct gpio_desc *reset_gpio = data->reset_gpio;
+
+	if (++data->cmd_timeout_cnt < 5)
+		return;
+
+	if (!reset_gpio) {
+		bt_dev_err(hdev, "No way to reset. Ignoring and continuing");
+		return;
+	}
+
+	/*
+	 * Toggle the hard reset line if the platform provides one. The reset
+	 * is going to yank the device off the USB and then replug. So doing
+	 * once is enough. The cleanup is handled correctly on the way out
+	 * (standard USB disconnect), and the new device is detected cleanly
+	 * and bound to the driver again like it should be.
+	 */
+	if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
+		bt_dev_err(hdev, "last reset failed? Not resetting again");
+		return;
+	}
+
+	bt_dev_err(hdev, "Initiating HW reset via gpio");
+	gpiod_set_value(reset_gpio, 1);
+	mdelay(100);
+	gpiod_set_value(reset_gpio, 0);
+}
+
 static inline void btusb_free_frags(struct btusb_data *data)
 {
 	unsigned long flags;
@@ -2915,6 +2952,7 @@ static int btusb_probe(struct usb_interface *intf,
 		       const struct usb_device_id *id)
 {
 	struct usb_endpoint_descriptor *ep_desc;
+	struct gpio_desc *reset_gpio;
 	struct btusb_data *data;
 	struct hci_dev *hdev;
 	unsigned ifnum_base;
@@ -3028,6 +3066,15 @@ static int btusb_probe(struct usb_interface *intf,
 
 	SET_HCIDEV_DEV(hdev, &intf->dev);
 
+	reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
+					GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpio)) {
+		err = PTR_ERR(reset_gpio);
+		goto out_free_dev;
+	} else if (reset_gpio) {
+		data->reset_gpio = reset_gpio;
+	}
+
 	hdev->open   = btusb_open;
 	hdev->close  = btusb_close;
 	hdev->flush  = btusb_flush;
@@ -3082,6 +3129,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->shutdown = btusb_shutdown_intel;
 		hdev->set_diag = btintel_set_diag_mfg;
 		hdev->set_bdaddr = btintel_set_bdaddr;
+		hdev->cmd_timeout = btusb_intel_cmd_timeout;
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
 		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
@@ -3094,6 +3142,7 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->hw_error = btintel_hw_error;
 		hdev->set_diag = btintel_set_diag;
 		hdev->set_bdaddr = btintel_set_bdaddr;
+		hdev->cmd_timeout = btusb_intel_cmd_timeout;
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
 		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 		set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
@@ -3226,6 +3275,8 @@ static int btusb_probe(struct usb_interface *intf,
 	return 0;
 
 out_free_dev:
+	if (data->reset_gpio)
+		gpiod_put(data->reset_gpio);
 	hci_free_dev(hdev);
 	return err;
 }
@@ -3269,6 +3320,9 @@ static void btusb_disconnect(struct usb_interface *intf)
 	if (data->oob_wake_irq)
 		device_init_wakeup(&data->udev->dev, false);
 
+	if (data->reset_gpio)
+		gpiod_put(data->reset_gpio);
+
 	hci_free_dev(hdev);
 }
 
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* [PATCH v6 2/4] usb: assign ACPI companions for embedded USB devices
From: Rajat Jain @ 2019-01-24 23:28 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
	David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
	linux-bluetooth, linux-kernel, linux-usb, netdev
  Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
	sukumar.ghorai
In-Reply-To: <20190124232814.252661-1-rajatja@google.com>

From: Dmitry Torokhov <dtor@chromium.org>

USB devices permanently connected to USB ports may be described in ACPI
tables and share ACPI devices with ports they are connected to. See [1]
for details.

This will allow us to describe sideband resources for devices, such as,
for example, hard reset line for BT USB controllers.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices

Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Rajat Jain <rajatja@google.com> (changed how we get the usb_port)
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
---
v6: same as v4
v5: same as v4
v4: Add Acked-by and Tested-by in signatures.
v3: same as v1
v2: same as v1

 drivers/usb/core/usb-acpi.c | 44 +++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 8ff73c83e8e8..9043d7242d67 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -200,30 +200,56 @@ static struct acpi_device *
 usb_acpi_find_companion_for_device(struct usb_device *udev)
 {
 	struct acpi_device *adev;
+	struct usb_port *port_dev;
+	struct usb_hub *hub;
+
+	if (!udev->parent) {
+		/* root hub is only child (_ADR=0) under its parent, the HC */
+		adev = ACPI_COMPANION(udev->dev.parent);
+		return acpi_find_child_device(adev, 0, false);
+	}
 
-	if (!udev->parent)
+	hub = usb_hub_to_struct_hub(udev->parent);
+	if (!hub)
 		return NULL;
 
-	/* root hub is only child (_ADR=0) under its parent, the HC */
-	adev = ACPI_COMPANION(udev->dev.parent);
-	return acpi_find_child_device(adev, 0, false);
+	/*
+	 * This is an embedded USB device connected to a port and such
+	 * devices share port's ACPI companion.
+	 */
+	port_dev = hub->ports[udev->portnum - 1];
+	return usb_acpi_get_companion_for_port(port_dev);
 }
 
-
 static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 {
 	/*
-	 * In the ACPI DSDT table, only usb root hub and usb ports are
-	 * acpi device nodes. The hierarchy like following.
+	 * The USB hierarchy like following:
+	 *
 	 * Device (EHC1)
 	 *	Device (HUBN)
 	 *		Device (PR01)
 	 *			Device (PR11)
 	 *			Device (PR12)
+	 *				Device (FN12)
+	 *				Device (FN13)
 	 *			Device (PR13)
 	 *			...
-	 * So all binding process is divided into two parts. binding
-	 * root hub and usb ports.
+	 * where HUBN is root hub, and PRNN are USB ports and devices
+	 * connected to them, and FNNN are individualk functions for
+	 * connected composite USB devices. PRNN and FNNN may contain
+	 * _CRS and other methods describing sideband resources for
+	 * the connected device.
+	 *
+	 * On the kernel side both root hub and embedded USB devices are
+	 * represented as instances of usb_device structure, and ports
+	 * are represented as usb_port structures, so the whole process
+	 * is split into 2 parts: finding companions for devices and
+	 * finding companions for ports.
+	 *
+	 * Note that we do not handle individual functions of composite
+	 * devices yet, for that we would need to assign companions to
+	 * devices corresponding to USB interfaces.
 	 */
 	if (is_usb_device(dev))
 		return usb_acpi_find_companion_for_device(to_usb_device(dev));
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* [PATCH v6 1/4] usb: split code locating ACPI companion into port and device
From: Rajat Jain @ 2019-01-24 23:28 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Greg Kroah-Hartman,
	David S. Miller, Dmitry Torokhov, Rajat Jain, Alex Hung,
	linux-bluetooth, linux-kernel, linux-usb, netdev
  Cc: rajatxjain, dtor, raghuram.hegde, chethan.tumkur.narayan,
	sukumar.ghorai
In-Reply-To: <20181117010748.24347-1-rajatja@google.com>

From: Dmitry Torokhov <dtor@chromium.org>

In preparation for handling embedded USB devices let's split
usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and
usb_acpi_find_companion_for_port().

Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Rajat Jain <rajatja@google.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
---
v6: same as v4
v5: same as v4
v4: Add Acked-by and Tested-by in signatures.
v3: same as v1
v2: same as v1

 drivers/usb/core/usb-acpi.c | 133 +++++++++++++++++++-----------------
 1 file changed, 72 insertions(+), 61 deletions(-)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index e221861b3187..8ff73c83e8e8 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -139,12 +139,79 @@ static struct acpi_device *usb_acpi_find_port(struct acpi_device *parent,
 	return acpi_find_child_device(parent, raw, false);
 }
 
-static struct acpi_device *usb_acpi_find_companion(struct device *dev)
+static struct acpi_device *
+usb_acpi_get_companion_for_port(struct usb_port *port_dev)
 {
 	struct usb_device *udev;
 	struct acpi_device *adev;
 	acpi_handle *parent_handle;
+	int port1;
+
+	/* Get the struct usb_device point of port's hub */
+	udev = to_usb_device(port_dev->dev.parent->parent);
+
+	/*
+	 * The root hub ports' parent is the root hub. The non-root-hub
+	 * ports' parent is the parent hub port which the hub is
+	 * connected to.
+	 */
+	if (!udev->parent) {
+		adev = ACPI_COMPANION(&udev->dev);
+		port1 = usb_hcd_find_raw_port_number(bus_to_hcd(udev->bus),
+						     port_dev->portnum);
+	} else {
+		parent_handle = usb_get_hub_port_acpi_handle(udev->parent,
+							     udev->portnum);
+		if (!parent_handle)
+			return NULL;
+
+		acpi_bus_get_device(parent_handle, &adev);
+		port1 = port_dev->portnum;
+	}
+
+	return usb_acpi_find_port(adev, port1);
+}
+
+static struct acpi_device *
+usb_acpi_find_companion_for_port(struct usb_port *port_dev)
+{
+	struct acpi_device *adev;
+	struct acpi_pld_info *pld;
+	acpi_handle *handle;
+	acpi_status status;
+
+	adev = usb_acpi_get_companion_for_port(port_dev);
+	if (!adev)
+		return NULL;
+
+	handle = adev->handle;
+	status = acpi_get_physical_device_location(handle, &pld);
+	if (!ACPI_FAILURE(status) && pld) {
+		port_dev->location = USB_ACPI_LOCATION_VALID
+			| pld->group_token << 8 | pld->group_position;
+		port_dev->connect_type = usb_acpi_get_connect_type(handle, pld);
+		ACPI_FREE(pld);
+	}
 
+	return adev;
+}
+
+static struct acpi_device *
+usb_acpi_find_companion_for_device(struct usb_device *udev)
+{
+	struct acpi_device *adev;
+
+	if (!udev->parent)
+		return NULL;
+
+	/* root hub is only child (_ADR=0) under its parent, the HC */
+	adev = ACPI_COMPANION(udev->dev.parent);
+	return acpi_find_child_device(adev, 0, false);
+}
+
+
+static struct acpi_device *usb_acpi_find_companion(struct device *dev)
+{
 	/*
 	 * In the ACPI DSDT table, only usb root hub and usb ports are
 	 * acpi device nodes. The hierarchy like following.
@@ -158,66 +225,10 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 	 * So all binding process is divided into two parts. binding
 	 * root hub and usb ports.
 	 */
-	if (is_usb_device(dev)) {
-		udev = to_usb_device(dev);
-		if (udev->parent)
-			return NULL;
-
-		/* root hub is only child (_ADR=0) under its parent, the HC */
-		adev = ACPI_COMPANION(dev->parent);
-		return acpi_find_child_device(adev, 0, false);
-	} else if (is_usb_port(dev)) {
-		struct usb_port *port_dev = to_usb_port(dev);
-		int port1 = port_dev->portnum;
-		struct acpi_pld_info *pld;
-		acpi_handle *handle;
-		acpi_status status;
-
-		/* Get the struct usb_device point of port's hub */
-		udev = to_usb_device(dev->parent->parent);
-
-		/*
-		 * The root hub ports' parent is the root hub. The non-root-hub
-		 * ports' parent is the parent hub port which the hub is
-		 * connected to.
-		 */
-		if (!udev->parent) {
-			struct usb_hcd *hcd = bus_to_hcd(udev->bus);
-			int raw;
-
-			raw = usb_hcd_find_raw_port_number(hcd, port1);
-
-			adev = usb_acpi_find_port(ACPI_COMPANION(&udev->dev),
-						  raw);
-
-			if (!adev)
-				return NULL;
-		} else {
-			parent_handle =
-				usb_get_hub_port_acpi_handle(udev->parent,
-				udev->portnum);
-			if (!parent_handle)
-				return NULL;
-
-			acpi_bus_get_device(parent_handle, &adev);
-
-			adev = usb_acpi_find_port(adev, port1);
-
-			if (!adev)
-				return NULL;
-		}
-		handle = adev->handle;
-		status = acpi_get_physical_device_location(handle, &pld);
-		if (ACPI_FAILURE(status) || !pld)
-			return adev;
-
-		port_dev->location = USB_ACPI_LOCATION_VALID
-			| pld->group_token << 8 | pld->group_position;
-		port_dev->connect_type = usb_acpi_get_connect_type(handle, pld);
-		ACPI_FREE(pld);
-
-		return adev;
-	}
+	if (is_usb_device(dev))
+		return usb_acpi_find_companion_for_device(to_usb_device(dev));
+	else if (is_usb_port(dev))
+		return usb_acpi_find_companion_for_port(to_usb_port(dev));
 
 	return NULL;
 }
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* Re: [PATCH net-next 1/7] net: tls: Save iv in tls_rec for async crypto requests
From: Dave Watson @ 2019-01-24 22:34 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev@vger.kernel.org, Dave Miller, Vakul Garg, Boris Pismenny,
	Aviad Yehezkel, John Fastabend, Daniel Borkmann
In-Reply-To: <20190124141452.062d6d36@cakuba.netronome.com>

On 01/24/19 02:14 PM, Jakub Kicinski wrote:
> On Thu, 24 Jan 2019 17:20:12 +0000, Dave Watson wrote:
> > aead_request_set_crypt takes an iv pointer, and we change the iv
> > soon after setting it.  Some async crypto algorithms don't save the iv,
> > so we need to save it in the tls_rec for async requests.
> > 
> > Found by hardcoding x64 aesni to use async crypto manager (to test the async
> > codepath), however I don't think this combination can happen in the wild.
> > Presumably other hardware offloads will need this fix, but there have been
> > no user reports.
> > 
> > Fixes: a42055e8d2c30 ("Add support for async encryption of records...")
> > Signed-off-by: Dave Watson <davejwatson@fb.com>
> 
> This one should target net, no?

Later patches touch the same code, so would need to be in both to
avoid merge conflicts.

^ permalink raw reply

* [Patch net] netrom: switch to sock timer API
From: Cong Wang @ 2019-01-24 22:18 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, syzbot+defa700d16f1bd1b9a05, Ralf Baechle, linux-hams

sk_reset_timer() and sk_stop_timer() properly handle
sock refcnt for timer function. Switching to them
could fix a refcounting bug reported by syzbot.

Reported-and-tested-by: syzbot+defa700d16f1bd1b9a05@syzkaller.appspotmail.com
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-hams@vger.kernel.org
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/netrom/nr_timer.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/netrom/nr_timer.c b/net/netrom/nr_timer.c
index cbd51ed5a2d7..908e53ab47a4 100644
--- a/net/netrom/nr_timer.c
+++ b/net/netrom/nr_timer.c
@@ -52,21 +52,21 @@ void nr_start_t1timer(struct sock *sk)
 {
 	struct nr_sock *nr = nr_sk(sk);
 
-	mod_timer(&nr->t1timer, jiffies + nr->t1);
+	sk_reset_timer(sk, &nr->t1timer, jiffies + nr->t1);
 }
 
 void nr_start_t2timer(struct sock *sk)
 {
 	struct nr_sock *nr = nr_sk(sk);
 
-	mod_timer(&nr->t2timer, jiffies + nr->t2);
+	sk_reset_timer(sk, &nr->t2timer, jiffies + nr->t2);
 }
 
 void nr_start_t4timer(struct sock *sk)
 {
 	struct nr_sock *nr = nr_sk(sk);
 
-	mod_timer(&nr->t4timer, jiffies + nr->t4);
+	sk_reset_timer(sk, &nr->t4timer, jiffies + nr->t4);
 }
 
 void nr_start_idletimer(struct sock *sk)
@@ -74,37 +74,37 @@ void nr_start_idletimer(struct sock *sk)
 	struct nr_sock *nr = nr_sk(sk);
 
 	if (nr->idle > 0)
-		mod_timer(&nr->idletimer, jiffies + nr->idle);
+		sk_reset_timer(sk, &nr->idletimer, jiffies + nr->idle);
 }
 
 void nr_start_heartbeat(struct sock *sk)
 {
-	mod_timer(&sk->sk_timer, jiffies + 5 * HZ);
+	sk_reset_timer(sk, &sk->sk_timer, jiffies + 5 * HZ);
 }
 
 void nr_stop_t1timer(struct sock *sk)
 {
-	del_timer(&nr_sk(sk)->t1timer);
+	sk_stop_timer(sk, &nr_sk(sk)->t1timer);
 }
 
 void nr_stop_t2timer(struct sock *sk)
 {
-	del_timer(&nr_sk(sk)->t2timer);
+	sk_stop_timer(sk, &nr_sk(sk)->t2timer);
 }
 
 void nr_stop_t4timer(struct sock *sk)
 {
-	del_timer(&nr_sk(sk)->t4timer);
+	sk_stop_timer(sk, &nr_sk(sk)->t4timer);
 }
 
 void nr_stop_idletimer(struct sock *sk)
 {
-	del_timer(&nr_sk(sk)->idletimer);
+	sk_stop_timer(sk, &nr_sk(sk)->idletimer);
 }
 
 void nr_stop_heartbeat(struct sock *sk)
 {
-	del_timer(&sk->sk_timer);
+	sk_stop_timer(sk, &sk->sk_timer);
 }
 
 int nr_t1timer_running(struct sock *sk)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next 1/7] net: tls: Save iv in tls_rec for async crypto requests
From: Jakub Kicinski @ 2019-01-24 22:14 UTC (permalink / raw)
  To: Dave Watson
  Cc: netdev@vger.kernel.org, Dave Miller, Vakul Garg, Boris Pismenny,
	Aviad Yehezkel, John Fastabend, Daniel Borkmann
In-Reply-To: <20190124172146.q4acuud6z35wevm4@davejwatson-mba.dhcp.thefacebook.com>

On Thu, 24 Jan 2019 17:20:12 +0000, Dave Watson wrote:
> aead_request_set_crypt takes an iv pointer, and we change the iv
> soon after setting it.  Some async crypto algorithms don't save the iv,
> so we need to save it in the tls_rec for async requests.
> 
> Found by hardcoding x64 aesni to use async crypto manager (to test the async
> codepath), however I don't think this combination can happen in the wild.
> Presumably other hardware offloads will need this fix, but there have been
> no user reports.
> 
> Fixes: a42055e8d2c30 ("Add support for async encryption of records...")
> Signed-off-by: Dave Watson <davejwatson@fb.com>

This one should target net, no?

^ permalink raw reply

* [PATCH bpf-next v3 7/7] samples/bpf: Check the prog id before exiting
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Check the program id within the signal handler on polling xdp samples
that were previously converted to libbpf usage. Avoid the situation of
unloading the program that was not attached by sample that is exiting.

Reported-by: Michal Papaj <michal.papaj@intel.com>
Reported-by: Jakub Spizewski <jakub.spizewski@intel.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp1_user.c             | 19 +++++++++++++++-
 samples/bpf/xdp_adjust_tail_user.c  | 25 +++++++++++++++++----
 samples/bpf/xdp_redirect_cpu_user.c | 30 ++++++++++++++++++++------
 samples/bpf/xdp_redirect_map_user.c | 37 ++++++++++++++++++++++++++++---
 samples/bpf/xdp_redirect_user.c     | 38 +++++++++++++++++++++++++++++---
 samples/bpf/xdp_router_ipv4_user.c  | 43 ++++++++++++++++++++++---------------
 samples/bpf/xdp_rxq_info_user.c     | 28 +++++++++++++++++++-----
 samples/bpf/xdp_sample_pkts_user.c  | 29 ++++++++++++++++++++-----
 samples/bpf/xdp_tx_iptunnel_user.c  | 23 +++++++++++++++++---
 samples/bpf/xdpsock_user.c          | 18 +++++++++++++++-
 10 files changed, 242 insertions(+), 48 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 505bce207165..3acc0e1d589a 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -23,10 +23,17 @@
 
 static int ifindex;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	else
+		printf("program on interface changed, not removing\n");
 	exit(0);
 }
 
@@ -74,11 +81,14 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "FSN";
 	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
+	int err;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -139,6 +149,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	poll_stats(map_fd, 2);
 
 	return 0;
diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index 049bddf7778b..01fc700d6a0c 100644
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -25,11 +25,19 @@
 
 static int ifindex = -1;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		else
+			printf("program on interface changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -72,11 +80,14 @@ int main(int argc, char **argv)
 	};
 	unsigned char opt_flags[256] = {};
 	const char *optstr = "i:T:SNFh";
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	unsigned int kill_after_s = 0;
 	int i, prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
+	int err;
 
 	for (i = 0; i < strlen(optstr); i++)
 		if (optstr[i] != 'h' && 'a' <= optstr[i] && optstr[i] <= 'z')
@@ -146,9 +157,15 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	poll_stats(map_fd, kill_after_s);
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return 1;
+	}
+	prog_id = info.id;
 
-	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	poll_stats(map_fd, kill_after_s);
+	int_exit(0);
 
 	return 0;
 }
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 0224afb55845..d8ba9f3d2c63 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -32,6 +32,7 @@ static const char *__doc__ =
 static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
+static __u32 prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int cpu_map_fd;
@@ -68,11 +69,19 @@ static const struct option long_options[] = {
 
 static void int_exit(int sig)
 {
-	fprintf(stderr,
-		"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
-		ifindex, ifname);
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id) {
+			fprintf(stderr,
+				"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
+				ifindex, ifname);
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		} else {
+			printf("program on interface changed, not removing\n");
+		}
+	}
 	exit(EXIT_OK);
 }
 
@@ -608,6 +617,8 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_UNSPEC,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	bool use_separators = true;
 	bool stress_mode = false;
 	struct bpf_program *prog;
@@ -617,9 +628,9 @@ int main(int argc, char **argv)
 	int longindex = 0;
 	int interval = 2;
 	int add_cpu = -1;
+	int opt, err;
 	int prog_fd;
 	__u32 qsize;
-	int opt;
 
 	/* Notice: choosing he queue size is very important with the
 	 * ixgbe driver, because it's driver page recycling trick is
@@ -746,6 +757,13 @@ int main(int argc, char **argv)
 		return EXIT_FAIL_XDP;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	stats_poll(interval, use_separators, prog_name, stress_mode);
 	return EXIT_OK;
 }
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 470e1a7e8810..cae7b9cead74 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -29,15 +29,29 @@
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
+static __u32 prog_id;
+static __u32 dummy_prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
-	if (ifindex_out_xdp_dummy_attached)
-		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
+	else
+		printf("program on iface IN changed, not removing\n");
+
+	if (ifindex_out_xdp_dummy_attached) {
+		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id, xdp_flags);
+		if (dummy_prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+		else
+			printf("program on iface OUT changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -82,6 +96,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	struct bpf_program *prog, *dummy_prog;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int prog_fd, dummy_prog_fd;
 	const char *optstr = "FSN";
 	struct bpf_object *obj;
@@ -153,6 +169,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	prog_id = info.id;
+
 	/* Loading dummy XDP prog on out-device */
 	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
@@ -160,6 +183,14 @@ int main(int argc, char **argv)
 		ifindex_out_xdp_dummy_attached = false;
 	}
 
+	memset(&info, 0, sizeof(info));
+	ret = bpf_obj_get_info_by_fd(dummy_prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	dummy_prog_id = info.id;
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index be6058cda97c..230b1e5e7f61 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -29,15 +29,30 @@
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
+static __u32 prog_id;
+static __u32 dummy_prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
-	if (ifindex_out_xdp_dummy_attached)
-		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
+	else
+		printf("program on iface IN changed, not removing\n");
+
+	if (ifindex_out_xdp_dummy_attached) {
+		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id,
+				    xdp_flags);
+		if (dummy_prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+		else
+			printf("program on iface OUT changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -84,6 +99,8 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, tx_port_map_fd, opt;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	char filename[256];
@@ -154,6 +171,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	prog_id = info.id;
+
 	/* Loading dummy XDP prog on out-device */
 	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
@@ -161,6 +185,14 @@ int main(int argc, char **argv)
 		ifindex_out_xdp_dummy_attached = false;
 	}
 
+	memset(&info, 0, sizeof(info));
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	dummy_prog_id = info.id;
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index 208d6a996478..3991bd42b20c 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -30,7 +30,8 @@
 
 int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int total_ifindex;
-int *ifindex_list;
+static int *ifindex_list;
+static __u32 *prog_id_list;
 char buf[8192];
 static int lpm_map_fd;
 static int rxcnt_map_fd;
@@ -41,23 +42,26 @@ static int tx_port_map_fd;
 static int get_route_table(int rtm_family);
 static void int_exit(int sig)
 {
+	__u32 prog_id;
 	int i = 0;
 
-	for (i = 0; i < total_ifindex; i++)
-		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
+	for (i = 0; i < total_ifindex; i++) {
+		bpf_get_link_xdp_id(ifindex_list[i], &prog_id, flags);
+		if (prog_id_list[i] == prog_id)
+			bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
+		else
+			printf("program on iface %d changed, not removing\n",
+			       ifindex_list[i]);
+	}
 	exit(0);
 }
 
 static void close_and_exit(int sig)
 {
-	int i = 0;
-
 	close(sock);
 	close(sock_arp);
 
-	for (i = 0; i < total_ifindex; i++)
-		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
-	exit(0);
+	int_exit(0);
 }
 
 /* Get the mac address of the interface given interface name */
@@ -186,13 +190,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
 		route.iface_name = alloca(sizeof(char *) * IFNAMSIZ);
 		route.iface_name = if_indextoname(route.iface, route.iface_name);
 		route.mac = getmac(route.iface_name);
-		if (route.mac == -1) {
-			int i = 0;
-
-			for (i = 0; i < total_ifindex; i++)
-				bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
-			exit(0);
-		}
+		if (route.mac == -1)
+			int_exit(0);
 		assert(bpf_map_update_elem(tx_port_map_fd,
 					   &route.iface, &route.iface, 0) == 0);
 		if (rtm_family == AF_INET) {
@@ -625,12 +624,14 @@ int main(int ac, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "SF";
 	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
 	int prog_fd, opt;
-	int i = 1;
+	int err, i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	prog_load_attr.file = filename;
@@ -687,7 +688,7 @@ int main(int ac, char **argv)
 		return 1;
 	}
 
-	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
+	ifindex_list = (int *)calloc(total_ifindex, sizeof(int *));
 	for (i = 0; i < total_ifindex; i++) {
 		ifindex_list[i] = if_nametoindex(ifname_list[i]);
 		if (!ifindex_list[i]) {
@@ -696,6 +697,7 @@ int main(int ac, char **argv)
 			return 1;
 		}
 	}
+	prog_id_list = (__u32 *)calloc(total_ifindex, sizeof(__u32 *));
 	for (i = 0; i < total_ifindex; i++) {
 		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) < 0) {
 			printf("link set xdp fd failed\n");
@@ -706,6 +708,13 @@ int main(int ac, char **argv)
 
 			return 1;
 		}
+		err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+		if (err) {
+			printf("can't get prog info - %s\n", strerror(errno));
+			return err;
+		}
+		prog_id_list[i] = info.id;
+		memset(&info, 0, sizeof(info));
 		printf("Attached to %d\n", ifindex_list[i]);
 	}
 	signal(SIGINT, int_exit);
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index e7a98c2a440f..7602a54eeba6 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -29,6 +29,7 @@ static const char *__doc__ = " XDP RX-queue info extract example\n\n"
 static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
+static __u32 prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
@@ -58,11 +59,19 @@ static const struct option long_options[] = {
 
 static void int_exit(int sig)
 {
-	fprintf(stderr,
-		"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
-		ifindex, ifname);
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id) {
+			fprintf(stderr,
+				"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
+				ifindex, ifname);
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		} else {
+			printf("program on interface changed, not removing\n");
+		}
+	}
 	exit(EXIT_OK);
 }
 
@@ -447,6 +456,8 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int prog_fd, map_fd, opt, err;
 	bool use_separators = true;
 	struct config cfg = { 0 };
@@ -580,6 +591,13 @@ int main(int argc, char **argv)
 		return EXIT_FAIL_XDP;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	stats_poll(interval, action, cfg_options);
 	return EXIT_OK;
 }
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 362ad35b524d..dcf78fbc371e 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -24,25 +24,44 @@ static int pmu_fds[MAX_CPUS], if_idx;
 static struct perf_event_mmap_page *headers[MAX_CPUS];
 static char *if_name;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static int do_attach(int idx, int fd, const char *name)
 {
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int err;
 
 	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
-	if (err < 0)
+	if (err < 0) {
 		printf("ERROR: failed to attach program to %s\n", name);
+		return err;
+	}
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
 
 	return err;
 }
 
 static int do_detach(int idx, const char *name)
 {
-	int err;
+	__u32 curr_prog_id;
+	int err = 0;
 
-	err = bpf_set_link_xdp_fd(idx, -1, 0);
-	if (err < 0)
-		printf("ERROR: failed to detach program from %s\n", name);
+	bpf_get_link_xdp_id(idx, &curr_prog_id, 0);
+
+	if (prog_id == curr_prog_id) {
+		err = bpf_set_link_xdp_fd(idx, -1, 0);
+		if (err < 0)
+			printf("ERROR: failed to detach prog from %s\n", name);
+	} else {
+		printf("program on interface changed, not removing\n");
+	}
 
 	return err;
 }
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index e3de60930d27..4c1b9b14aa79 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -27,11 +27,19 @@
 static int ifindex = -1;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		else
+			printf("program on interface changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -148,13 +156,15 @@ int main(int argc, char **argv)
 	int min_port = 0, max_port = 0, vip2tnl_map_fd;
 	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
 	unsigned char opt_flags[256] = {};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	unsigned int kill_after_s = 0;
 	struct iptnl_info tnl = {};
 	struct bpf_object *obj;
 	struct vip vip = {};
 	char filename[256];
 	int opt, prog_fd;
-	int i;
+	int i, err;
 
 	tnl.family = AF_UNSPEC;
 	vip.protocol = IPPROTO_TCP;
@@ -276,6 +286,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	poll_stats(kill_after_s);
 
 	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 188723784768..d7fb74d9a223 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -76,6 +76,7 @@ static int opt_poll;
 static int opt_shared_packet_buffer;
 static int opt_interval = 1;
 static u32 opt_xdp_bind_flags;
+static __u32 prog_id;
 
 struct xdp_umem_uqueue {
 	u32 cached_prod;
@@ -631,9 +632,15 @@ static void *poller(void *arg)
 
 static void int_exit(int sig)
 {
+	__u32 curr_prog_id;
+
 	(void)sig;
 	dump_stats();
-	bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
+	bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
+	else
+		printf("program on interface changed, not removing\n");
 	exit(EXIT_SUCCESS);
 }
 
@@ -907,6 +914,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	int prog_fd, qidconf_map, xsks_map;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	struct bpf_object *obj;
 	char xdp_filename[256];
 	struct bpf_map *map;
@@ -953,6 +962,13 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return 1;
+	}
+	prog_id = info.id;
+
 	ret = bpf_map_update_elem(qidconf_map, &key, &opt_queue, 0);
 	if (ret) {
 		fprintf(stderr, "ERROR: bpf_map_update_elem qidconf\n");
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 6/7] libbpf: Add a support for getting xdp prog id on ifindex
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Since we have a dedicated netlink attributes for xdp setup on a
particular interface, it is now possible to retrieve the program id that
is currently attached to the interface. The use case is targeted for
sample xdp programs, which will store the program id just after loading
bpf program onto iface. On shutdown, the sample will make sure that it
can unload the program by querying again the iface and verifying that
both program id's matches.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/lib/bpf/libbpf.h   |  1 +
 tools/lib/bpf/libbpf.map |  1 +
 tools/lib/bpf/netlink.c  | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 931be6f3408c..43c77e98df6f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -317,6 +317,7 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
 			     struct bpf_object **pobj, int *prog_fd);
 
 LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
+LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
 
 enum bpf_perf_event_ret {
 	LIBBPF_PERF_EVENT_DONE	= 0,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index b183c6c3b990..d0e023a75d72 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -131,4 +131,5 @@ LIBBPF_0.0.2 {
 		bpf_probe_map_type;
 		bpf_probe_prog_type;
 		bpf_object__find_map_fd_by_name;
+		bpf_get_link_xdp_id;
 } LIBBPF_0.0.1;
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 0ce67aea8f3b..e44a6ef25678 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -21,6 +21,12 @@
 typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, libbpf_dump_nlmsg_t,
 			      void *cookie);
 
+struct xdp_id_md {
+	int ifindex;
+	__u32 flags;
+	__u32 id;
+};
+
 int libbpf_netlink_open(__u32 *nl_pid)
 {
 	struct sockaddr_nl sa;
@@ -196,6 +202,84 @@ static int __dump_link_nlmsg(struct nlmsghdr *nlh,
 	return dump_link_nlmsg(cookie, ifi, tb);
 }
 
+static unsigned char get_xdp_id_attr(unsigned char mode, __u32 flags)
+{
+	if (mode != XDP_ATTACHED_MULTI)
+		return IFLA_XDP_PROG_ID;
+	if (flags & XDP_FLAGS_DRV_MODE)
+		return IFLA_XDP_DRV_PROG_ID;
+	if (flags & XDP_FLAGS_HW_MODE)
+		return IFLA_XDP_HW_PROG_ID;
+	if (flags & XDP_FLAGS_SKB_MODE)
+		return IFLA_XDP_SKB_PROG_ID;
+
+	return IFLA_XDP_UNSPEC;
+}
+
+static int get_xdp_id(void *cookie, void *msg, struct nlattr **tb)
+{
+	struct nlattr *xdp_tb[IFLA_XDP_MAX + 1];
+	struct xdp_id_md *xdp_id = cookie;
+	struct ifinfomsg *ifinfo = msg;
+	unsigned char mode, xdp_attr;
+	int ret;
+
+	if (xdp_id->ifindex && xdp_id->ifindex != ifinfo->ifi_index)
+		return 0;
+
+	if (!tb[IFLA_XDP])
+		return 0;
+
+	ret = libbpf_nla_parse_nested(xdp_tb, IFLA_XDP_MAX, tb[IFLA_XDP], NULL);
+	if (ret)
+		return ret;
+
+	if (!xdp_tb[IFLA_XDP_ATTACHED])
+		return 0;
+
+	mode = libbpf_nla_getattr_u8(xdp_tb[IFLA_XDP_ATTACHED]);
+	if (mode == XDP_ATTACHED_NONE)
+		return 0;
+
+	xdp_attr = get_xdp_id_attr(mode, xdp_id->flags);
+	if (!xdp_attr || !xdp_tb[xdp_attr])
+		return -ENOENT;
+
+	xdp_id->id = libbpf_nla_getattr_u32(xdp_tb[xdp_attr]);
+
+	return 0;
+}
+
+int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
+{
+	struct xdp_id_md xdp_id = {};
+	int sock, ret;
+	__u32 nl_pid;
+	__u32 mask;
+
+	if (flags & ~XDP_FLAGS_MASK)
+		return -EINVAL;
+
+	/* Check whether the single {HW,DRV,SKB} mode is set */
+	flags &= XDP_FLAGS_MODES;
+	mask = flags - 1;
+	if (flags && flags & mask)
+		return -EINVAL;
+
+	sock = libbpf_netlink_open(&nl_pid);
+	if (sock < 0)
+		return sock;
+
+	xdp_id.ifindex = ifindex;
+	xdp_id.flags = flags;
+
+	ret = libbpf_nl_get_link(sock, nl_pid, get_xdp_id, &xdp_id);
+	*prog_id = xdp_id.id;
+
+	close(sock);
+	return ret;
+}
+
 int libbpf_nl_get_link(int sock, unsigned int nl_pid,
 		       libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
 {
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 3/7] samples/bpf: Convert XDP samples to libbpf usage
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Some of XDP samples that are attaching the bpf program to the interface
via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
loading and manipulating the ebpf program and maps. Convert them to do
this through libbpf usage and remove bpf_load from the picture.

While at it remove what looks like debug leftover in
xdp_redirect_map_user.c

In xdp_redirect_cpu, change the way that the program to be loaded onto
interface is chosen - user now needs to pass the program's section name
instead of the relative number. In case of typo print out the section
names to choose from.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/Makefile                |   8 +-
 samples/bpf/xdp_redirect_cpu_user.c | 145 +++++++++++++++++++++++++-----------
 samples/bpf/xdp_redirect_map_user.c |  47 ++++++++----
 samples/bpf/xdp_redirect_user.c     |  44 ++++++++---
 samples/bpf/xdp_router_ipv4_user.c  |  75 +++++++++++++------
 samples/bpf/xdp_tx_iptunnel_user.c  |  37 ++++++---
 6 files changed, 253 insertions(+), 103 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 66ae15f27c70..4486fedaf09a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -87,18 +87,18 @@ test_cgrp2_sock2-objs := bpf_load.o test_cgrp2_sock2.o
 xdp1-objs := xdp1_user.o
 # reuse xdp1 source intentionally
 xdp2-objs := xdp1_user.o
-xdp_router_ipv4-objs := bpf_load.o xdp_router_ipv4_user.o
+xdp_router_ipv4-objs := xdp_router_ipv4_user.o
 test_current_task_under_cgroup-objs := bpf_load.o $(CGROUP_HELPERS) \
 				       test_current_task_under_cgroup_user.o
 trace_event-objs := bpf_load.o trace_event_user.o $(TRACE_HELPERS)
 sampleip-objs := bpf_load.o sampleip_user.o $(TRACE_HELPERS)
 tc_l2_redirect-objs := bpf_load.o tc_l2_redirect_user.o
 lwt_len_hist-objs := bpf_load.o lwt_len_hist_user.o
-xdp_tx_iptunnel-objs := bpf_load.o xdp_tx_iptunnel_user.o
+xdp_tx_iptunnel-objs := xdp_tx_iptunnel_user.o
 test_map_in_map-objs := bpf_load.o test_map_in_map_user.o
 per_socket_stats_example-objs := cookie_uid_helper_example.o
-xdp_redirect-objs := bpf_load.o xdp_redirect_user.o
-xdp_redirect_map-objs := bpf_load.o xdp_redirect_map_user.o
+xdp_redirect-objs := xdp_redirect_user.o
+xdp_redirect_map-objs := xdp_redirect_map_user.o
 xdp_redirect_cpu-objs := bpf_load.o xdp_redirect_cpu_user.o
 xdp_monitor-objs := bpf_load.o xdp_monitor_user.o
 xdp_rxq_info-objs := xdp_rxq_info_user.o
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index f141e752ca0a..8645ddc2da0e 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -24,12 +24,8 @@ static const char *__doc__ =
 /* How many xdp_progs are defined in _kern.c */
 #define MAX_PROG 6
 
-/* Wanted to get rid of bpf_load.h and fake-"libbpf.h" (and instead
- * use bpf/libbpf.h), but cannot as (currently) needed for XDP
- * attaching to a device via bpf_set_link_xdp_fd()
- */
 #include <bpf/bpf.h>
-#include "bpf_load.h"
+#include "bpf/libbpf.h"
 
 #include "bpf_util.h"
 
@@ -38,6 +34,15 @@ static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
 
 static __u32 xdp_flags;
+static int cpu_map_fd;
+static int rx_cnt_map_fd;
+static int redirect_err_cnt_map_fd;
+static int cpumap_enqueue_cnt_map_fd;
+static int cpumap_kthread_cnt_map_fd;
+static int cpus_available_map_fd;
+static int cpus_count_map_fd;
+static int cpus_iterator_map_fd;
+static int exception_cnt_map_fd;
 
 /* Exit return codes */
 #define EXIT_OK		0
@@ -52,7 +57,7 @@ static const struct option long_options[] = {
 	{"dev",		required_argument,	NULL, 'd' },
 	{"skb-mode",	no_argument,		NULL, 'S' },
 	{"sec",		required_argument,	NULL, 's' },
-	{"prognum",	required_argument,	NULL, 'p' },
+	{"progname",	required_argument,	NULL, 'p' },
 	{"qsize",	required_argument,	NULL, 'q' },
 	{"cpu",		required_argument,	NULL, 'c' },
 	{"stress-mode", no_argument,		NULL, 'x' },
@@ -70,7 +75,17 @@ static void int_exit(int sig)
 	exit(EXIT_OK);
 }
 
-static void usage(char *argv[])
+static void print_avail_progs(struct bpf_object *obj)
+{
+	struct bpf_program *pos;
+
+	bpf_object__for_each_program(pos, obj) {
+		if (bpf_program__is_xdp(pos))
+			printf(" %s\n", bpf_program__title(pos, false));
+	}
+}
+
+static void usage(char *argv[], struct bpf_object *obj)
 {
 	int i;
 
@@ -88,6 +103,8 @@ static void usage(char *argv[])
 				long_options[i].val);
 		printf("\n");
 	}
+	printf("\n Programs to be used for --progname:\n");
+	print_avail_progs(obj);
 	printf("\n");
 }
 
@@ -262,7 +279,7 @@ static __u64 calc_errs_pps(struct datarec *r,
 
 static void stats_print(struct stats_record *stats_rec,
 			struct stats_record *stats_prev,
-			int prog_num)
+			char *prog_name)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
 	double pps = 0, drop = 0, err = 0;
@@ -272,7 +289,7 @@ static void stats_print(struct stats_record *stats_rec,
 	int i;
 
 	/* Header */
-	printf("Running XDP/eBPF prog_num:%d\n", prog_num);
+	printf("Running XDP/eBPF prog_name:%s\n", prog_name);
 	printf("%-15s %-7s %-14s %-11s %-9s\n",
 	       "XDP-cpumap", "CPU:to", "pps", "drop-pps", "extra-info");
 
@@ -423,20 +440,20 @@ static void stats_collect(struct stats_record *rec)
 {
 	int fd, i;
 
-	fd = map_fd[1]; /* map: rx_cnt */
+	fd = rx_cnt_map_fd;
 	map_collect_percpu(fd, 0, &rec->rx_cnt);
 
-	fd = map_fd[2]; /* map: redirect_err_cnt */
+	fd = redirect_err_cnt_map_fd;
 	map_collect_percpu(fd, 1, &rec->redir_err);
 
-	fd = map_fd[3]; /* map: cpumap_enqueue_cnt */
+	fd = cpumap_enqueue_cnt_map_fd;
 	for (i = 0; i < MAX_CPUS; i++)
 		map_collect_percpu(fd, i, &rec->enq[i]);
 
-	fd = map_fd[4]; /* map: cpumap_kthread_cnt */
+	fd = cpumap_kthread_cnt_map_fd;
 	map_collect_percpu(fd, 0, &rec->kthread);
 
-	fd = map_fd[8]; /* map: exception_cnt */
+	fd = exception_cnt_map_fd;
 	map_collect_percpu(fd, 0, &rec->exception);
 }
 
@@ -461,7 +478,7 @@ static int create_cpu_entry(__u32 cpu, __u32 queue_size,
 	/* Add a CPU entry to cpumap, as this allocate a cpu entry in
 	 * the kernel for the cpu.
 	 */
-	ret = bpf_map_update_elem(map_fd[0], &cpu, &queue_size, 0);
+	ret = bpf_map_update_elem(cpu_map_fd, &cpu, &queue_size, 0);
 	if (ret) {
 		fprintf(stderr, "Create CPU entry failed (err:%d)\n", ret);
 		exit(EXIT_FAIL_BPF);
@@ -470,23 +487,22 @@ static int create_cpu_entry(__u32 cpu, __u32 queue_size,
 	/* Inform bpf_prog's that a new CPU is available to select
 	 * from via some control maps.
 	 */
-	/* map_fd[5] = cpus_available */
-	ret = bpf_map_update_elem(map_fd[5], &avail_idx, &cpu, 0);
+	ret = bpf_map_update_elem(cpus_available_map_fd, &avail_idx, &cpu, 0);
 	if (ret) {
 		fprintf(stderr, "Add to avail CPUs failed\n");
 		exit(EXIT_FAIL_BPF);
 	}
 
 	/* When not replacing/updating existing entry, bump the count */
-	/* map_fd[6] = cpus_count */
-	ret = bpf_map_lookup_elem(map_fd[6], &key, &curr_cpus_count);
+	ret = bpf_map_lookup_elem(cpus_count_map_fd, &key, &curr_cpus_count);
 	if (ret) {
 		fprintf(stderr, "Failed reading curr cpus_count\n");
 		exit(EXIT_FAIL_BPF);
 	}
 	if (new) {
 		curr_cpus_count++;
-		ret = bpf_map_update_elem(map_fd[6], &key, &curr_cpus_count, 0);
+		ret = bpf_map_update_elem(cpus_count_map_fd, &key,
+					  &curr_cpus_count, 0);
 		if (ret) {
 			fprintf(stderr, "Failed write curr cpus_count\n");
 			exit(EXIT_FAIL_BPF);
@@ -509,8 +525,8 @@ static void mark_cpus_unavailable(void)
 	int ret, i;
 
 	for (i = 0; i < MAX_CPUS; i++) {
-		/* map_fd[5] = cpus_available */
-		ret = bpf_map_update_elem(map_fd[5], &i, &invalid_cpu, 0);
+		ret = bpf_map_update_elem(cpus_available_map_fd, &i,
+					  &invalid_cpu, 0);
 		if (ret) {
 			fprintf(stderr, "Failed marking CPU unavailable\n");
 			exit(EXIT_FAIL_BPF);
@@ -530,7 +546,7 @@ static void stress_cpumap(void)
 	create_cpu_entry(1, 16000, 0, false);
 }
 
-static void stats_poll(int interval, bool use_separators, int prog_num,
+static void stats_poll(int interval, bool use_separators, char *prog_name,
 		       bool stress_mode)
 {
 	struct stats_record *record, *prev;
@@ -546,7 +562,7 @@ static void stats_poll(int interval, bool use_separators, int prog_num,
 	while (1) {
 		swap(&prev, &record);
 		stats_collect(record);
-		stats_print(record, prev, prog_num);
+		stats_print(record, prev, prog_name);
 		sleep(interval);
 		if (stress_mode)
 			stress_cpumap();
@@ -556,17 +572,51 @@ static void stats_poll(int interval, bool use_separators, int prog_num,
 	free_stats_record(prev);
 }
 
+static int init_map_fds(struct bpf_object *obj)
+{
+	cpu_map_fd = bpf_object__find_map_fd_by_name(obj, "cpu_map");
+	rx_cnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rx_cnt");
+	redirect_err_cnt_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "redirect_err_cnt");
+	cpumap_enqueue_cnt_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "cpumap_enqueue_cnt");
+	cpumap_kthread_cnt_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "cpumap_kthread_cnt");
+	cpus_available_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "cpus_available");
+	cpus_count_map_fd = bpf_object__find_map_fd_by_name(obj, "cpus_count");
+	cpus_iterator_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "cpus_iterator");
+	exception_cnt_map_fd =
+		bpf_object__find_map_fd_by_name(obj, "exception_cnt");
+
+	if (cpu_map_fd < 0 || rx_cnt_map_fd < 0 ||
+	    redirect_err_cnt_map_fd < 0 || cpumap_enqueue_cnt_map_fd < 0 ||
+	    cpumap_kthread_cnt_map_fd < 0 || cpus_available_map_fd < 0 ||
+	    cpus_count_map_fd < 0 || cpus_iterator_map_fd < 0 ||
+	    exception_cnt_map_fd < 0)
+		return -ENOENT;
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
+	char *prog_name = "xdp_cpu_map5_lb_hash_ip_pairs";
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_UNSPEC,
+	};
 	bool use_separators = true;
 	bool stress_mode = false;
+	struct bpf_program *prog;
+	struct bpf_object *obj;
 	char filename[256];
 	int added_cpus = 0;
 	int longindex = 0;
 	int interval = 2;
-	int prog_num = 5;
 	int add_cpu = -1;
+	int prog_fd;
 	__u32 qsize;
 	int opt;
 
@@ -579,22 +629,25 @@ int main(int argc, char **argv)
 	qsize = 128+64;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
 
 	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
 		perror("setrlimit(RLIMIT_MEMLOCK)");
 		return 1;
 	}
 
-	if (load_bpf_file(filename)) {
-		fprintf(stderr, "ERR in load_bpf_file(): %s", bpf_log_buf);
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return EXIT_FAIL;
-	}
 
-	if (!prog_fd[0]) {
-		fprintf(stderr, "ERR: load_bpf_file: %s\n", strerror(errno));
+	if (prog_fd < 0) {
+		fprintf(stderr, "ERR: bpf_prog_load_xattr: %s\n",
+			strerror(errno));
+		return EXIT_FAIL;
+	}
+	if (init_map_fds(obj) < 0) {
+		fprintf(stderr, "bpf_object__find_map_fd_by_name failed\n");
 		return EXIT_FAIL;
 	}
-
 	mark_cpus_unavailable();
 
 	/* Parse commands line args */
@@ -630,13 +683,7 @@ int main(int argc, char **argv)
 			break;
 		case 'p':
 			/* Selecting eBPF prog to load */
-			prog_num = atoi(optarg);
-			if (prog_num < 0 || prog_num >= MAX_PROG) {
-				fprintf(stderr,
-					"--prognum too large err(%d):%s\n",
-					errno, strerror(errno));
-				goto error;
-			}
+			prog_name = optarg;
 			break;
 		case 'c':
 			/* Add multiple CPUs */
@@ -656,21 +703,21 @@ int main(int argc, char **argv)
 		case 'h':
 		error:
 		default:
-			usage(argv);
+			usage(argv, obj);
 			return EXIT_FAIL_OPTION;
 		}
 	}
 	/* Required option */
 	if (ifindex == -1) {
 		fprintf(stderr, "ERR: required option --dev missing\n");
-		usage(argv);
+		usage(argv, obj);
 		return EXIT_FAIL_OPTION;
 	}
 	/* Required option */
 	if (add_cpu == -1) {
 		fprintf(stderr, "ERR: required option --cpu missing\n");
 		fprintf(stderr, " Specify multiple --cpu option to add more\n");
-		usage(argv);
+		usage(argv, obj);
 		return EXIT_FAIL_OPTION;
 	}
 
@@ -678,11 +725,23 @@ int main(int argc, char **argv)
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
-	if (bpf_set_link_xdp_fd(ifindex, prog_fd[prog_num], xdp_flags) < 0) {
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (!prog) {
+		fprintf(stderr, "bpf_object__find_program_by_title failed\n");
+		return EXIT_FAIL;
+	}
+
+	prog_fd = bpf_program__fd(prog);
+	if (prog_fd < 0) {
+		fprintf(stderr, "bpf_program__fd failed\n");
+		return EXIT_FAIL;
+	}
+
+	if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
 		fprintf(stderr, "link set xdp fd failed\n");
 		return EXIT_FAIL_XDP;
 	}
 
-	stats_poll(interval, use_separators, prog_num, stress_mode);
+	stats_poll(interval, use_separators, prog_name, stress_mode);
 	return EXIT_OK;
 }
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 4445e76854b5..60d46eea225b 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -22,15 +22,16 @@
 #include <libgen.h>
 #include <sys/resource.h>
 
-#include "bpf_load.h"
 #include "bpf_util.h"
 #include <bpf/bpf.h>
+#include "bpf/libbpf.h"
 
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
 static __u32 xdp_flags;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,7 @@ static void poll_stats(int interval, int ifindex)
 		int i;
 
 		sleep(interval);
-		assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+		assert(bpf_map_lookup_elem(rxcnt_map_fd, &key, values) == 0);
 		for (i = 0; i < nr_cpus; i++)
 			sum += (values[i] - prev[i]);
 		if (sum)
@@ -76,9 +77,16 @@ static void usage(const char *prog)
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_program *prog, *dummy_prog;
+	int prog_fd, dummy_prog_fd;
 	const char *optstr = "SN";
-	char filename[256];
+	struct bpf_object *obj;
 	int ret, opt, key = 0;
+	char filename[256];
+	int tx_port_map_fd;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -109,24 +117,40 @@ int main(int argc, char **argv)
 	printf("input: %d output: %d\n", ifindex_in, ifindex_out);
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
+
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
+		return 1;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	prog = bpf_program__next(NULL, obj);
+	dummy_prog = bpf_program__next(prog, obj);
+	if (!prog || !dummy_prog) {
+		printf("finding a prog in obj file failed\n");
+		return 1;
+	}
+	/* bpf_prog_load_xattr gives us the pointer to first prog's fd,
+	 * so we're missing only the fd for dummy prog
+	 */
+	dummy_prog_fd = bpf_program__fd(dummy_prog);
+	if (prog_fd < 0 || dummy_prog_fd < 0) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
 
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	if (tx_port_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
 		return 1;
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd, xdp_flags) < 0) {
 		printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
 		return 1;
 	}
 
 	/* Loading dummy XDP prog on out-device */
-	if (bpf_set_link_xdp_fd(ifindex_out, prog_fd[1],
+	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
 		printf("WARN: link set xdp fd failed on %d\n", ifindex_out);
 		ifindex_out_xdp_dummy_attached = false;
@@ -135,11 +159,8 @@ int main(int argc, char **argv)
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
-	printf("map[0] (vports) = %i, map[1] (map) = %i, map[2] (count) = %i\n",
-		map_fd[0], map_fd[1], map_fd[2]);
-
 	/* populate virtual to physical port map */
-	ret = bpf_map_update_elem(map_fd[0], &key, &ifindex_out, 0);
+	ret = bpf_map_update_elem(tx_port_map_fd, &key, &ifindex_out, 0);
 	if (ret) {
 		perror("bpf_update_elem");
 		goto out;
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index 81a69e36cb78..93404820df68 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -22,15 +22,16 @@
 #include <libgen.h>
 #include <sys/resource.h>
 
-#include "bpf_load.h"
 #include "bpf_util.h"
 #include <bpf/bpf.h>
+#include "bpf/libbpf.h"
 
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
 static __u32 xdp_flags;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,7 @@ static void poll_stats(int interval, int ifindex)
 		int i;
 
 		sleep(interval);
-		assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+		assert(bpf_map_lookup_elem(rxcnt_map_fd, &key, values) == 0);
 		for (i = 0; i < nr_cpus; i++)
 			sum += (values[i] - prev[i]);
 		if (sum)
@@ -77,9 +78,16 @@ static void usage(const char *prog)
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_program *prog, *dummy_prog;
+	int prog_fd, tx_port_map_fd, opt;
 	const char *optstr = "SN";
+	struct bpf_object *obj;
 	char filename[256];
-	int ret, opt, key = 0;
+	int dummy_prog_fd;
+	int ret, key = 0;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -110,24 +118,40 @@ int main(int argc, char **argv)
 	printf("input: %d output: %d\n", ifindex_in, ifindex_out);
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
+		return 1;
+
+	prog = bpf_program__next(NULL, obj);
+	dummy_prog = bpf_program__next(prog, obj);
+	if (!prog || !dummy_prog) {
+		printf("finding a prog in obj file failed\n");
+		return 1;
+	}
+	/* bpf_prog_load_xattr gives us the pointer to first prog's fd,
+	 * so we're missing only the fd for dummy prog
+	 */
+	dummy_prog_fd = bpf_program__fd(dummy_prog);
+	if (prog_fd < 0 || dummy_prog_fd < 0) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
 
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	if (tx_port_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
 		return 1;
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd, xdp_flags) < 0) {
 		printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
 		return 1;
 	}
 
 	/* Loading dummy XDP prog on out-device */
-	if (bpf_set_link_xdp_fd(ifindex_out, prog_fd[1],
+	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
 		printf("WARN: link set xdp fd failed on %d\n", ifindex_out);
 		ifindex_out_xdp_dummy_attached = false;
@@ -137,7 +161,7 @@ int main(int argc, char **argv)
 	signal(SIGTERM, int_exit);
 
 	/* bpf redirect port */
-	ret = bpf_map_update_elem(map_fd[0], &key, &ifindex_out, 0);
+	ret = bpf_map_update_elem(tx_port_map_fd, &key, &ifindex_out, 0);
 	if (ret) {
 		perror("bpf_update_elem");
 		goto out;
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index b2b4dfa776c8..cea2306f5ab7 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -15,7 +15,6 @@
 #include <string.h>
 #include <sys/socket.h>
 #include <unistd.h>
-#include "bpf_load.h"
 #include <bpf/bpf.h>
 #include <arpa/inet.h>
 #include <fcntl.h>
@@ -25,11 +24,17 @@
 #include <sys/ioctl.h>
 #include <sys/syscall.h>
 #include "bpf_util.h"
+#include "bpf/libbpf.h"
 
 int sock, sock_arp, flags = 0;
 static int total_ifindex;
 int *ifindex_list;
 char buf[8192];
+static int lpm_map_fd;
+static int rxcnt_map_fd;
+static int arp_table_map_fd;
+static int exact_match_map_fd;
+static int tx_port_map_fd;
 
 static int get_route_table(int rtm_family);
 static void int_exit(int sig)
@@ -186,7 +191,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
 				bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
 			exit(0);
 		}
-		assert(bpf_map_update_elem(map_fd[4], &route.iface, &route.iface, 0) == 0);
+		assert(bpf_map_update_elem(tx_port_map_fd,
+					   &route.iface, &route.iface, 0) == 0);
 		if (rtm_family == AF_INET) {
 			struct trie_value {
 				__u8 prefix[4];
@@ -207,11 +213,16 @@ static void read_route(struct nlmsghdr *nh, int nll)
 			direct_entry.arp.dst = 0;
 			if (route.dst_len == 32) {
 				if (nh->nlmsg_type == RTM_DELROUTE) {
-					assert(bpf_map_delete_elem(map_fd[3], &route.dst) == 0);
+					assert(bpf_map_delete_elem(exact_match_map_fd,
+								   &route.dst) == 0);
 				} else {
-					if (bpf_map_lookup_elem(map_fd[2], &route.dst, &direct_entry.arp.mac) == 0)
+					if (bpf_map_lookup_elem(arp_table_map_fd,
+								&route.dst,
+								&direct_entry.arp.mac) == 0)
 						direct_entry.arp.dst = route.dst;
-					assert(bpf_map_update_elem(map_fd[3], &route.dst, &direct_entry, 0) == 0);
+					assert(bpf_map_update_elem(exact_match_map_fd,
+								   &route.dst,
+								   &direct_entry, 0) == 0);
 				}
 			}
 			for (i = 0; i < 4; i++)
@@ -225,7 +236,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 			       route.gw, route.dst_len,
 			       route.metric,
 			       route.iface_name);
-			if (bpf_map_lookup_elem(map_fd[0], prefix_key,
+			if (bpf_map_lookup_elem(lpm_map_fd, prefix_key,
 						prefix_value) < 0) {
 				for (i = 0; i < 4; i++)
 					prefix_value->prefix[i] = prefix_key->data[i];
@@ -234,7 +245,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 				prefix_value->gw = route.gw;
 				prefix_value->metric = route.metric;
 
-				assert(bpf_map_update_elem(map_fd[0],
+				assert(bpf_map_update_elem(lpm_map_fd,
 							   prefix_key,
 							   prefix_value, 0
 							   ) == 0);
@@ -247,7 +258,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 					       prefix_key->data[2],
 					       prefix_key->data[3],
 					       prefix_key->prefixlen);
-					assert(bpf_map_delete_elem(map_fd[0],
+					assert(bpf_map_delete_elem(lpm_map_fd,
 								   prefix_key
 								   ) == 0);
 					/* Rereading the route table to check if
@@ -275,8 +286,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 					prefix_value->ifindex = route.iface;
 					prefix_value->gw = route.gw;
 					prefix_value->metric = route.metric;
-					assert(bpf_map_update_elem(
-								   map_fd[0],
+					assert(bpf_map_update_elem(lpm_map_fd,
 								   prefix_key,
 								   prefix_value,
 								   0) == 0);
@@ -401,7 +411,8 @@ static void read_arp(struct nlmsghdr *nh, int nll)
 		arp_entry.mac = atol(mac);
 		printf("%x\t\t%llx\n", arp_entry.dst, arp_entry.mac);
 		if (ndm_family == AF_INET) {
-			if (bpf_map_lookup_elem(map_fd[3], &arp_entry.dst,
+			if (bpf_map_lookup_elem(exact_match_map_fd,
+						&arp_entry.dst,
 						&direct_entry) == 0) {
 				if (nh->nlmsg_type == RTM_DELNEIGH) {
 					direct_entry.arp.dst = 0;
@@ -410,16 +421,17 @@ static void read_arp(struct nlmsghdr *nh, int nll)
 					direct_entry.arp.dst = arp_entry.dst;
 					direct_entry.arp.mac = arp_entry.mac;
 				}
-				assert(bpf_map_update_elem(map_fd[3],
+				assert(bpf_map_update_elem(exact_match_map_fd,
 							   &arp_entry.dst,
 							   &direct_entry, 0
 							   ) == 0);
 				memset(&direct_entry, 0, sizeof(direct_entry));
 			}
 			if (nh->nlmsg_type == RTM_DELNEIGH) {
-				assert(bpf_map_delete_elem(map_fd[2], &arp_entry.dst) == 0);
+				assert(bpf_map_delete_elem(arp_table_map_fd,
+							   &arp_entry.dst) == 0);
 			} else if (nh->nlmsg_type == RTM_NEWNEIGH) {
-				assert(bpf_map_update_elem(map_fd[2],
+				assert(bpf_map_update_elem(arp_table_map_fd,
 							   &arp_entry.dst,
 							   &arp_entry.mac, 0
 							   ) == 0);
@@ -553,7 +565,8 @@ static int monitor_route(void)
 		for (key = 0; key < nr_keys; key++) {
 			__u64 sum = 0;
 
-			assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+			assert(bpf_map_lookup_elem(rxcnt_map_fd,
+						   &key, values) == 0);
 			for (i = 0; i < nr_cpus; i++)
 				sum += (values[i] - prev[key][i]);
 			if (sum)
@@ -596,11 +609,18 @@ static int monitor_route(void)
 
 int main(int ac, char **argv)
 {
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
+	int prog_fd;
 	int i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
+
 	if (ac < 2) {
 		printf("usage: %s [-S] Interface name list\n", argv[0]);
 		return 1;
@@ -614,15 +634,28 @@ int main(int ac, char **argv)
 		total_ifindex = ac - 1;
 		ifname_list = (argv + 1);
 	}
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
-	}
+
 	printf("\n**************loading bpf file*********************\n\n\n");
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	if (!prog_fd) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
+
+	lpm_map_fd = bpf_object__find_map_fd_by_name(obj, "lpm_map");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	arp_table_map_fd = bpf_object__find_map_fd_by_name(obj, "arp_table");
+	exact_match_map_fd = bpf_object__find_map_fd_by_name(obj,
+							     "exact_match");
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	if (lpm_map_fd < 0 || rxcnt_map_fd < 0 || arp_table_map_fd < 0 ||
+	    exact_match_map_fd < 0 || tx_port_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
+		return 1;
+	}
+
 	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
 	for (i = 0; i < total_ifindex; i++) {
 		ifindex_list[i] = if_nametoindex(ifname_list[i]);
@@ -633,7 +666,7 @@ int main(int ac, char **argv)
 		}
 	}
 	for (i = 0; i < total_ifindex; i++) {
-		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd[0], flags) < 0) {
+		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) < 0) {
 			printf("link set xdp fd failed\n");
 			int recovery_index = i;
 
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index a4ccc33adac0..5093d8220da5 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -17,7 +17,7 @@
 #include <netinet/ether.h>
 #include <unistd.h>
 #include <time.h>
-#include "bpf_load.h"
+#include "bpf/libbpf.h"
 #include <bpf/bpf.h>
 #include "bpf_util.h"
 #include "xdp_tx_iptunnel_common.h"
@@ -26,6 +26,7 @@
 
 static int ifindex = -1;
 static __u32 xdp_flags = 0;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,8 @@ static void poll_stats(unsigned int kill_after_s)
 		for (proto = 0; proto < nr_protos; proto++) {
 			__u64 sum = 0;
 
-			assert(bpf_map_lookup_elem(map_fd[0], &proto, values) == 0);
+			assert(bpf_map_lookup_elem(rxcnt_map_fd, &proto,
+						   values) == 0);
 			for (i = 0; i < nr_cpus; i++)
 				sum += (values[i] - prev[proto][i]);
 
@@ -138,15 +140,19 @@ static int parse_ports(const char *port_str, int *min_port, int *max_port)
 
 int main(int argc, char **argv)
 {
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	int min_port = 0, max_port = 0, vip2tnl_map_fd;
+	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
 	unsigned char opt_flags[256] = {};
 	unsigned int kill_after_s = 0;
-	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
-	int min_port = 0, max_port = 0;
 	struct iptnl_info tnl = {};
-	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_object *obj;
 	struct vip vip = {};
 	char filename[256];
-	int opt;
+	int opt, prog_fd;
 	int i;
 
 	tnl.family = AF_UNSPEC;
@@ -232,29 +238,36 @@ int main(int argc, char **argv)
 	}
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
-	}
 
-	if (!prog_fd[0]) {
+	if (!prog_fd) {
 		printf("load_bpf_file: %s\n", strerror(errno));
 		return 1;
 	}
 
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	vip2tnl_map_fd = bpf_object__find_map_fd_by_name(obj, "vip2tnl");
+	if (vip2tnl_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
+		return 1;
+	}
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
 	while (min_port <= max_port) {
 		vip.dport = htons(min_port++);
-		if (bpf_map_update_elem(map_fd[1], &vip, &tnl, BPF_NOEXIST)) {
+		if (bpf_map_update_elem(vip2tnl_map_fd, &vip, &tnl,
+					BPF_NOEXIST)) {
 			perror("bpf_map_update_elem(&vip2tnl)");
 			return 1;
 		}
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 5/7] samples/bpf: Add a "force" flag to XDP samples
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Make xdp samples consistent with iproute2 behavior and set the
XDP_FLAGS_UPDATE_IF_NOEXIST by default when setting the xdp program on
interface. Provide an option for user to force the program loading,
which as a result will not include the mentioned flag in
bpf_set_link_xdp_fd call.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp1_user.c             | 10 +++++---
 samples/bpf/xdp_adjust_tail_user.c  |  8 ++++--
 samples/bpf/xdp_redirect_cpu_user.c |  8 ++++--
 samples/bpf/xdp_redirect_map_user.c | 10 +++++---
 samples/bpf/xdp_redirect_user.c     | 10 +++++---
 samples/bpf/xdp_router_ipv4_user.c  | 50 +++++++++++++++++++++++++++----------
 samples/bpf/xdp_rxq_info_user.c     |  8 ++++--
 samples/bpf/xdp_sample_pkts_user.c  | 40 +++++++++++++++++++++++------
 samples/bpf/xdp_tx_iptunnel_user.c  |  8 ++++--
 samples/bpf/xdpsock_user.c          |  7 ++++--
 10 files changed, 119 insertions(+), 40 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 8bfda95c77ad..505bce207165 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -22,7 +22,7 @@
 #include "bpf/libbpf.h"
 
 static int ifindex;
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static void int_exit(int sig)
 {
@@ -63,7 +63,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFACE\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -73,7 +74,7 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
@@ -87,6 +88,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index 3042ce37dae8..049bddf7778b 100644
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -24,7 +24,7 @@
 #define STATS_INTERVAL_S 2U
 
 static int ifindex = -1;
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static void int_exit(int sig)
 {
@@ -60,6 +60,7 @@ static void usage(const char *cmd)
 	printf("    -T <stop-after-X-seconds> Default: 0 (forever)\n");
 	printf("    -S use skb-mode\n");
 	printf("    -N enforce native mode\n");
+	printf("    -F force loading prog\n");
 	printf("    -h Display this help\n");
 }
 
@@ -70,8 +71,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	unsigned char opt_flags[256] = {};
+	const char *optstr = "i:T:SNFh";
 	unsigned int kill_after_s = 0;
-	const char *optstr = "i:T:SNh";
 	int i, prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
@@ -96,6 +97,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(argv[0]);
 			return 1;
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 8645ddc2da0e..0224afb55845 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -33,7 +33,7 @@ static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int cpu_map_fd;
 static int rx_cnt_map_fd;
 static int redirect_err_cnt_map_fd;
@@ -62,6 +62,7 @@ static const struct option long_options[] = {
 	{"cpu",		required_argument,	NULL, 'c' },
 	{"stress-mode", no_argument,		NULL, 'x' },
 	{"no-separators", no_argument,		NULL, 'z' },
+	{"force",	no_argument,		NULL, 'F' },
 	{0, 0, NULL,  0 }
 };
 
@@ -651,7 +652,7 @@ int main(int argc, char **argv)
 	mark_cpus_unavailable();
 
 	/* Parse commands line args */
-	while ((opt = getopt_long(argc, argv, "hSd:",
+	while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzF",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -700,6 +701,9 @@ int main(int argc, char **argv)
 		case 'q':
 			qsize = atoi(optarg);
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		case 'h':
 		error:
 		default:
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 60d46eea225b..470e1a7e8810 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -30,7 +30,7 @@ static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -70,7 +70,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFINDEX_IN IFINDEX_OUT\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -82,7 +83,7 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, dummy_prog_fd;
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	int ret, opt, key = 0;
 	char filename[256];
@@ -96,6 +97,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index 93404820df68..be6058cda97c 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -30,7 +30,7 @@ static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -70,7 +70,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFINDEX_IN IFINDEX_OUT\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -83,7 +84,7 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, tx_port_map_fd, opt;
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	char filename[256];
 	int dummy_prog_fd;
@@ -97,6 +98,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index c63c6beec7d6..208d6a996478 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -26,8 +26,9 @@
 #include "bpf_util.h"
 #include "bpf/libbpf.h"
 #include <sys/resource.h>
+#include <libgen.h>
 
-int sock, sock_arp, flags = 0;
+int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int total_ifindex;
 int *ifindex_list;
 char buf[8192];
@@ -608,33 +609,56 @@ static int monitor_route(void)
 	return ret;
 }
 
+static void usage(const char *prog)
+{
+	fprintf(stderr,
+		"%s: %s [OPTS] interface name list\n\n"
+		"OPTS:\n"
+		"    -S    use skb-mode\n"
+		"    -F    force loading prog\n",
+		__func__, prog);
+}
+
 int main(int ac, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	const char *optstr = "SF";
 	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
-	int prog_fd;
+	int prog_fd, opt;
 	int i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	prog_load_attr.file = filename;
 
-	if (ac < 2) {
-		printf("usage: %s [-S] Interface name list\n", argv[0]);
-		return 1;
+	total_ifindex = ac - 1;
+	ifname_list = (argv + 1);
+
+	while ((opt = getopt(ac, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'S':
+			flags |= XDP_FLAGS_SKB_MODE;
+			total_ifindex--;
+			ifname_list++;
+			break;
+		case 'F':
+			flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			total_ifindex--;
+			ifname_list++;
+			break;
+		default:
+			usage(basename(argv[0]));
+			return 1;
+		}
 	}
-	if (!strcmp(argv[1], "-S")) {
-		flags = XDP_FLAGS_SKB_MODE;
-		total_ifindex = ac - 2;
-		ifname_list = (argv + 2);
-	} else {
-		flags = 0;
-		total_ifindex = ac - 1;
-		ifname_list = (argv + 1);
+
+	if (optind == ac) {
+		usage(basename(argv[0]));
+		return 1;
 	}
 
 	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index ef26f882f92f..e7a98c2a440f 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -30,7 +30,7 @@ static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static struct bpf_map *stats_global_map;
 static struct bpf_map *rx_queue_index_map;
@@ -52,6 +52,7 @@ static const struct option long_options[] = {
 	{"action",	required_argument,	NULL, 'a' },
 	{"readmem", 	no_argument,		NULL, 'r' },
 	{"swapmac", 	no_argument,		NULL, 'm' },
+	{"force",	no_argument,		NULL, 'F' },
 	{0, 0, NULL,  0 }
 };
 
@@ -487,7 +488,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Parse commands line args */
-	while ((opt = getopt_long(argc, argv, "hSd:",
+	while ((opt = getopt_long(argc, argv, "FhSrmzd:s:a:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -524,6 +525,9 @@ int main(int argc, char **argv)
 		case 'm':
 			cfg_options |= SWAP_MAC;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		case 'h':
 		error:
 		default:
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 5f5828ee0761..362ad35b524d 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -13,6 +13,8 @@
 #include <libbpf.h>
 #include <bpf/bpf.h>
 #include <sys/resource.h>
+#include <libgen.h>
+#include <linux/if_link.h>
 
 #include "perf-sys.h"
 #include "trace_helpers.h"
@@ -21,12 +23,13 @@
 static int pmu_fds[MAX_CPUS], if_idx;
 static struct perf_event_mmap_page *headers[MAX_CPUS];
 static char *if_name;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static int do_attach(int idx, int fd, const char *name)
 {
 	int err;
 
-	err = bpf_set_link_xdp_fd(idx, fd, 0);
+	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
 	if (err < 0)
 		printf("ERROR: failed to attach program to %s\n", name);
 
@@ -98,21 +101,42 @@ static void sig_handler(int signo)
 	exit(0);
 }
 
+static void usage(const char *prog)
+{
+	fprintf(stderr,
+		"%s: %s [OPTS] IFINDEX\n\n"
+		"OPTS:\n"
+		"    -F    force loading prog\n",
+		__func__, prog);
+}
+
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	const char *optstr = "F";
+	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
-	int prog_fd, map_fd;
 	char filename[256];
 	int ret, err, i;
 	int numcpus;
 
-	if (argc < 2) {
-		printf("Usage: %s <ifname>\n", argv[0]);
+	while ((opt = getopt(argc, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
+		default:
+			usage(basename(argv[0]));
+			return 1;
+		}
+	}
+
+	if (optind == argc) {
+		usage(basename(argv[0]));
 		return 1;
 	}
 
@@ -143,16 +167,16 @@ int main(int argc, char **argv)
 	}
 	map_fd = bpf_map__fd(map);
 
-	if_idx = if_nametoindex(argv[1]);
+	if_idx = if_nametoindex(argv[optind]);
 	if (!if_idx)
-		if_idx = strtoul(argv[1], NULL, 0);
+		if_idx = strtoul(argv[optind], NULL, 0);
 
 	if (!if_idx) {
 		fprintf(stderr, "Invalid ifname\n");
 		return 1;
 	}
-	if_name = argv[1];
-	err = do_attach(if_idx, prog_fd, argv[1]);
+	if_name = argv[optind];
+	err = do_attach(if_idx, prog_fd, if_name);
 	if (err)
 		return err;
 
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index 5093d8220da5..e3de60930d27 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -25,7 +25,7 @@
 #define STATS_INTERVAL_S 2U
 
 static int ifindex = -1;
-static __u32 xdp_flags = 0;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -83,6 +83,7 @@ static void usage(const char *cmd)
 	printf("    -P <IP-Protocol> Default is TCP\n");
 	printf("    -S use skb-mode\n");
 	printf("    -N enforce native mode\n");
+	printf("    -F Force loading the XDP prog\n");
 	printf("    -h Display this help\n");
 }
 
@@ -145,7 +146,7 @@ int main(int argc, char **argv)
 	};
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	int min_port = 0, max_port = 0, vip2tnl_map_fd;
-	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
+	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
 	unsigned char opt_flags[256] = {};
 	unsigned int kill_after_s = 0;
 	struct iptnl_info tnl = {};
@@ -217,6 +218,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(argv[0]);
 			return 1;
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 57ecadc58403..188723784768 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -68,7 +68,7 @@ enum benchmark_type {
 };
 
 static enum benchmark_type opt_bench = BENCH_RXDROP;
-static u32 opt_xdp_flags;
+static u32 opt_xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static const char *opt_if = "";
 static int opt_ifindex;
 static int opt_queue;
@@ -682,7 +682,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "rtli:q:psSNn:cz", long_options,
+		c = getopt_long(argc, argv, "Frtli:q:psSNn:cz", long_options,
 				&option_index);
 		if (c == -1)
 			break;
@@ -725,6 +725,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'c':
 			opt_xdp_bind_flags |= XDP_COPY;
 			break;
+		case 'F':
+			opt_xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 		}
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 4/7] samples/bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4}
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

There is a common problem with xdp samples that happens when user wants
to run a particular sample and some bpf program is already loaded. The
default 64kb RLIMIT_MEMLOCK resource limit will cause a following error
(assuming that xdp sample that is failing was converted to libbpf
usage):

libbpf: Error in bpf_object__probe_name():Operation not permitted(1).
Couldn't load basic 'r0 = 0' BPF program.
libbpf: failed to load object './xdp_sample_pkts_kern.o'

Fix it in xdp_sample_pkts and xdp_router_ipv4 by setting RLIMIT_MEMLOCK
to RLIM_INFINITY.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp_router_ipv4_user.c | 7 +++++++
 samples/bpf/xdp_sample_pkts_user.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index cea2306f5ab7..c63c6beec7d6 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -25,6 +25,7 @@
 #include <sys/syscall.h>
 #include "bpf_util.h"
 #include "bpf/libbpf.h"
+#include <sys/resource.h>
 
 int sock, sock_arp, flags = 0;
 static int total_ifindex;
@@ -609,6 +610,7 @@ static int monitor_route(void)
 
 int main(int ac, char **argv)
 {
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
@@ -635,6 +637,11 @@ int main(int ac, char **argv)
 		ifname_list = (argv + 1);
 	}
 
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
 	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
 
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 8dd87c1eb560..5f5828ee0761 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -12,6 +12,7 @@
 #include <signal.h>
 #include <libbpf.h>
 #include <bpf/bpf.h>
+#include <sys/resource.h>
 
 #include "perf-sys.h"
 #include "trace_helpers.h"
@@ -99,6 +100,7 @@ static void sig_handler(int signo)
 
 int main(int argc, char **argv)
 {
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
@@ -114,6 +116,11 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
 	numcpus = get_nprocs();
 	if (numcpus > MAX_CPUS)
 		numcpus = MAX_CPUS;
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 2/7] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Jesper Dangaard Brouer <brouer@redhat.com>

The sample xdp_redirect_cpu is not using helper bpf_trace_printk.
Thus it makes no sense that the --debug option us reading
from /sys/kernel/debug/tracing/trace_pipe via read_trace_pipe.
Simply remove it.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 samples/bpf/xdp_redirect_cpu_user.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 2d23054aaccf..f141e752ca0a 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -51,7 +51,6 @@ static const struct option long_options[] = {
 	{"help",	no_argument,		NULL, 'h' },
 	{"dev",		required_argument,	NULL, 'd' },
 	{"skb-mode",	no_argument,		NULL, 'S' },
-	{"debug",	no_argument,		NULL, 'D' },
 	{"sec",		required_argument,	NULL, 's' },
 	{"prognum",	required_argument,	NULL, 'p' },
 	{"qsize",	required_argument,	NULL, 'q' },
@@ -563,7 +562,6 @@ int main(int argc, char **argv)
 	bool use_separators = true;
 	bool stress_mode = false;
 	char filename[256];
-	bool debug = false;
 	int added_cpus = 0;
 	int longindex = 0;
 	int interval = 2;
@@ -624,9 +622,6 @@ int main(int argc, char **argv)
 		case 'S':
 			xdp_flags |= XDP_FLAGS_SKB_MODE;
 			break;
-		case 'D':
-			debug = true;
-			break;
 		case 'x':
 			stress_mode = true;
 			break;
@@ -688,11 +683,6 @@ int main(int argc, char **argv)
 		return EXIT_FAIL_XDP;
 	}
 
-	if (debug) {
-		printf("Debug-mode reading trace pipe (fix #define DEBUG)\n");
-		read_trace_pipe();
-	}
-
 	stats_poll(interval, use_separators, prog_num, stress_mode);
 	return EXIT_OK;
 }
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 1/7] libbpf: Add a helper for retrieving a map fd for a given name
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer
In-Reply-To: <20190124214747.6986-1-maciejromanfijalkowski@gmail.com>

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

XDP samples are mostly cooperating with eBPF maps through their file
descriptors. In case of a eBPF program that contains multiple maps it
might be tiresome to iterate through them and call bpf_map__fd for each
one. Add a helper mostly based on bpf_object__find_map_by_name, but
instead of returning the struct bpf_map pointer, return map fd.

Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/lib/bpf/libbpf.c   | 6 ++++++
 tools/lib/bpf/libbpf.h   | 3 +++
 tools/lib/bpf/libbpf.map | 1 +
 3 files changed, 10 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2ccde17957e6..03bc01ca2577 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2884,6 +2884,12 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
 	return NULL;
 }
 
+int
+bpf_object__find_map_fd_by_name(struct bpf_object *obj, const char *name)
+{
+	return bpf_map__fd(bpf_object__find_map_by_name(obj, name));
+}
+
 struct bpf_map *
 bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 62ae6cb93da1..931be6f3408c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -264,6 +264,9 @@ struct bpf_map;
 LIBBPF_API struct bpf_map *
 bpf_object__find_map_by_name(struct bpf_object *obj, const char *name);
 
+LIBBPF_API int
+bpf_object__find_map_fd_by_name(struct bpf_object *obj, const char *name);
+
 /*
  * Get bpf_map through the offset of corresponding struct bpf_map_def
  * in the BPF object file.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 266bc95d0142..b183c6c3b990 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -130,4 +130,5 @@ LIBBPF_0.0.2 {
 		bpf_probe_helper;
 		bpf_probe_map_type;
 		bpf_probe_prog_type;
+		bpf_object__find_map_fd_by_name;
 } LIBBPF_0.0.1;
-- 
2.16.1


^ permalink raw reply related

* [PATCH bpf-next v3 0/7] xdp: Avoid unloading xdp prog not attached by sample
From: Maciej Fijalkowski @ 2019-01-24 21:47 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Hi!
This patchset tries to address the situation where:
* user loads a particular xdp sample application that does stats polling
* user loads another sample application on the same interface
* then, user sends SIGINT/SIGTERM to the app that was attached as a first one
* second application ends up with an unloaded xdp program

1st patch contains a helper libbpf function for getting the map fd by a
given map name.
In patch 2 Jesper removes the read_trace_pipe usage from xdp_redirect_cpu which
was a blocker for converting this sample to libbpf usage.
3rd patch updates a bunch of xdp samples to make the use of libbpf.
Patch 4 adjusts RLIMIT_MEMLOCK for two samples touched in this patchset.
Patch 5 makes the samples behavior similar to what iproute2 does when loading
xdp prog - the "force" flag is introduced.
Patch 6 introduces the libbpf function that will query the driver from
userspace about the currently attached xdp prog id.

Use it in samples that do polling by checking the prog id in signal handler
and comparing it with previously stored one which is the scope of patch 7.

Thanks!

v1->v2:
* add a libbpf helper for getting a prog via relative index
* include xdp_redirect_cpu into conversion

v2->v3: mostly addressing Daniel's/Jesper's comments
* get rid of the helper from v1->v2
* feed the xdp_redirect_cpu with program name instead of number
* rebase

Jesper Dangaard Brouer (1):
  samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe

Maciej Fijalkowski (6):
  libbpf: Add a helper for retrieving a map fd for a given name
  samples/bpf: Convert XDP samples to libbpf usage
  samples/bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4}
  samples/bpf: Add a "force" flag to XDP samples
  libbpf: Add a support for getting xdp prog id on ifindex
  samples/bpf: Check the prog id before exiting

 samples/bpf/Makefile                |   8 +-
 samples/bpf/xdp1_user.c             |  29 +++++-
 samples/bpf/xdp_adjust_tail_user.c  |  33 +++++--
 samples/bpf/xdp_redirect_cpu_user.c | 191 +++++++++++++++++++++++++-----------
 samples/bpf/xdp_redirect_map_user.c |  94 ++++++++++++++----
 samples/bpf/xdp_redirect_user.c     |  92 ++++++++++++++---
 samples/bpf/xdp_router_ipv4_user.c  | 171 +++++++++++++++++++++++---------
 samples/bpf/xdp_rxq_info_user.c     |  36 +++++--
 samples/bpf/xdp_sample_pkts_user.c  |  76 +++++++++++---
 samples/bpf/xdp_tx_iptunnel_user.c  |  66 ++++++++++---
 samples/bpf/xdpsock_user.c          |  25 ++++-
 tools/lib/bpf/libbpf.c              |   6 ++
 tools/lib/bpf/libbpf.h              |   4 +
 tools/lib/bpf/libbpf.map            |   2 +
 tools/lib/bpf/netlink.c             |  84 ++++++++++++++++
 15 files changed, 720 insertions(+), 197 deletions(-)

-- 
2.16.1


^ permalink raw reply

* Re: marvell 6190 NAT performance
From: Florian Fainelli @ 2019-01-24 21:31 UTC (permalink / raw)
  To: Marek Behun; +Cc: Andrew Lunn, netdev, Russell King
In-Reply-To: <20190124212640.43813222@nic.cz>

On 1/24/19 12:26 PM, Marek Behun wrote:
> Hello,
> 
> I am encountering strange performance issue when benchmarking NAT
> performance on Armada 3720 with Marvell 88e6190 switch.
> 
> Download speed (from internet, via Armada 3720 NAT, via switch to LAN
> device) is ~750mbps and the CPU running on 100% (mostly in ksoftirq).
> Upload speed is ~250mbps.
> 
> When the LAN device is connected to A3720 directly (via SFP), the
> speeds are both ~1000mbps.

OK and that presumably uses the second Ethernet MAC on the SoC right?

> 
> I realize that packing/unpacking packets with Marvell header for the
> switch takes some time, but is such a performance drop expected?

If you run perf top/record you would be able to see that pretty quickly,
I would not think that processing of the Marvell DSA tag would incur
such a high penalty though since the packets are already hot in D$ by
the time we get to mangle them for the DSA network devices.

How about pure (non-NAT) IP routing? How about just bridging between WAN
and LAN?

> 
> This was tested with 5.0.0-rc2 and also 4.14.
> 
> Thank you.
> 
> Marek
> 

-- 
Florian

^ permalink raw reply

* [PATCH iproute2-next 2/2] tc: replace left side comparison
From: Stephen Hemminger @ 2019-01-24 20:30 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <20190124203014.7450-1-stephen@networkplumber.org>

The kernel (and iproute2) don't use the if (NULL == x) style
and instead prefer if (!x)

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/m_action.c | 2 +-
 tc/m_ipt.c    | 3 +--
 tc/m_xt_old.c | 3 +--
 tc/tc_util.c  | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index d5fd5affe703..b5aff3ab4896 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -363,7 +363,7 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
 
 	parse_rtattr_nested(tb, tot_acts, arg);
 
-	if (tab_flush && NULL != tb[0]  && NULL == tb[1])
+	if (tab_flush && tb[0] && !tb[1])
 		return tc_print_action_flush(f, tb[0]);
 
 	open_json_array(PRINT_JSON, "actions");
diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index b48cc0a9c85f..1d73cb98895a 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -299,9 +299,8 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 		int i;
 
 		for (i = 0; i < rargc; i++) {
-			if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) {
+			if (!argv[i] || strcmp(argv[i], "action") == 0)
 				break;
-			}
 		}
 		iargc = argc = i;
 	}
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 313bea61cc7e..25d367785786 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -224,9 +224,8 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 		int i;
 
 		for (i = 0; i < rargc; i++) {
-			if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) {
+			if (!argv[i] || strcmp(argv[i], "action") == 0)
 				break;
-			}
 		}
 		iargc = argc = i;
 	}
diff --git a/tc/tc_util.c b/tc/tc_util.c
index ab717890bb2a..1dfdae141f4d 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -857,7 +857,7 @@ void print_tcstats_attr(FILE *fp, struct rtattr *tb[], char *prefix, struct rtat
 
 	if (tb[TCA_STATS2]) {
 		print_tcstats2_attr(fp, tb[TCA_STATS2], prefix, xstats);
-		if (xstats && NULL == *xstats)
+		if (xstats && !*xstats)
 			goto compat_xstats;
 		return;
 	}
-- 
2.17.1


^ permalink raw reply related

* [PATCH iproute2-next 1/2] tc: replace print_string with fprintf for error messages
From: Stephen Hemminger @ 2019-01-24 20:30 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

Any error output should go to standard error.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/f_matchall.c | 2 +-
 tc/m_connmark.c | 2 +-
 tc/m_csum.c     | 2 +-
 tc/m_gact.c     | 2 +-
 tc/m_ife.c      | 2 +-
 tc/m_mirred.c   | 2 +-
 tc/m_nat.c      | 2 +-
 tc/m_sample.c   | 2 +-
 tc/m_skbedit.c  | 2 +-
 tc/m_vlan.c     | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tc/f_matchall.c b/tc/f_matchall.c
index 03dd51de1ace..59f452c5361e 100644
--- a/tc/f_matchall.c
+++ b/tc/f_matchall.c
@@ -146,7 +146,7 @@ static int matchall_print_opt(struct filter_util *qu, FILE *f,
 
 	if (tb[TCA_MATCHALL_PCNT]) {
 		if (RTA_PAYLOAD(tb[TCA_MATCHALL_PCNT])  < sizeof(*pf)) {
-			print_string(PRINT_FP, NULL, "Broken perf counters\n", NULL);
+			fprintf(stderr, "Performance counter size mismatch\n");
 			return -1;
 		}
 		pf = RTA_DATA(tb[TCA_MATCHALL_PCNT]);
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 45e2d05f1a91..34d80b1a27c0 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -114,7 +114,7 @@ static int print_connmark(struct action_util *au, FILE *f, struct rtattr *arg)
 
 	parse_rtattr_nested(tb, TCA_CONNMARK_MAX, arg);
 	if (tb[TCA_CONNMARK_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL connmark parameters]");
+		fprintf(stderr, "NULL connmark parameters\n");
 		return -1;
 	}
 
diff --git a/tc/m_csum.c b/tc/m_csum.c
index 752269d1d020..450f2b557b44 100644
--- a/tc/m_csum.c
+++ b/tc/m_csum.c
@@ -172,7 +172,7 @@ print_csum(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_CSUM_MAX, arg);
 
 	if (tb[TCA_CSUM_PARMS] == NULL) {
-		fprintf(f, "[NULL csum parameters]");
+		fprintf(stderr, "missing csum parameters\n");
 		return -1;
 	}
 	sel = RTA_DATA(tb[TCA_CSUM_PARMS]);
diff --git a/tc/m_gact.c b/tc/m_gact.c
index a0a3c33d23da..342927bc2ed7 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -174,7 +174,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_GACT_MAX, arg);
 
 	if (tb[TCA_GACT_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL gact parameters]");
+		fprintf(stderr, "[NULL gact parameters]\n");
 		return -1;
 	}
 	p = RTA_DATA(tb[TCA_GACT_PARMS]);
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 2bf9f2047b46..b754cfc3a03a 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -234,7 +234,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_IFE_MAX, arg);
 
 	if (tb[TCA_IFE_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL ife parameters]");
+		fprintf(stderr, "[NULL ife parameters]\n");
 		return -1;
 	}
 	p = RTA_DATA(tb[TCA_IFE_PARMS]);
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index c7f7318b8413..fb4cab5fb9eb 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -286,7 +286,7 @@ print_mirred(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_MIRRED_MAX, arg);
 
 	if (tb[TCA_MIRRED_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL mirred parameters]");
+		fprintf(stderr, "mirred parameters missing\n");
 		return -1;
 	}
 	p = RTA_DATA(tb[TCA_MIRRED_PARMS]);
diff --git a/tc/m_nat.c b/tc/m_nat.c
index ee0b7520a605..4cd255299ccb 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -152,7 +152,7 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_NAT_MAX, arg);
 
 	if (tb[TCA_NAT_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL nat parameters]");
+		fprintf(stderr, "[NULL nat parameters]\n");
 		return -1;
 	}
 	sel = RTA_DATA(tb[TCA_NAT_PARMS]);
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 39a99246a8ea..2af098ecba90 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -149,7 +149,7 @@ static int print_sample(struct action_util *au, FILE *f, struct rtattr *arg)
 
 	if (!tb[TCA_SAMPLE_PARMS] || !tb[TCA_SAMPLE_RATE] ||
 	    !tb[TCA_SAMPLE_PSAMPLE_GROUP]) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL sample parameters]");
+		fprintf(stderr, "[NULL sample parameters]\n");
 		return -1;
 	}
 	p = RTA_DATA(tb[TCA_SAMPLE_PARMS]);
diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index b6b839f8ef6c..ee0cd987fa3a 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -186,7 +186,7 @@ static int print_skbedit(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_SKBEDIT_MAX, arg);
 
 	if (tb[TCA_SKBEDIT_PARMS] == NULL) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL skbedit parameters]");
+		fprintf(stderr, "[NULL skbedit parameters]\n");
 		return -1;
 	}
 	p = RTA_DATA(tb[TCA_SKBEDIT_PARMS]);
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 412f6aa1000e..f58bbf0a0320 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -188,7 +188,7 @@ static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_VLAN_MAX, arg);
 
 	if (!tb[TCA_VLAN_PARMS]) {
-		print_string(PRINT_FP, NULL, "%s", "[NULL vlan parameters]");
+		fprintf(stderr, "[NULL vlan parameters]\n");
 		return -1;
 	}
 	parm = RTA_DATA(tb[TCA_VLAN_PARMS]);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH iproute2-next] iproute: lwtunnel set ip tunnel flags
From: Stephen Hemminger @ 2019-01-24 20:27 UTC (permalink / raw)
  To: wenxu; +Cc: dsahern, netdev
In-Reply-To: <1548338627-28538-1-git-send-email-wenxu@ucloud.cn>

On Thu, 24 Jan 2019 22:03:47 +0800
wenxu@ucloud.cn wrote:

> From: wenxu <wenxu@ucloud.cn>
> 
> ip l add dev tun type gretap external
> ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 dev gretap
> 
> For gretap example when the command set the id but don't set the
> TUNNEL_KEY flags. There is no key field in the send packet
> 
> User can set flags with key, csum, seq
> ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 key csum dev gretap
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>

Applied

^ permalink raw reply

* marvell 6190 NAT performance
From: Marek Behun @ 2019-01-24 20:26 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, netdev, Russell King

Hello,

I am encountering strange performance issue when benchmarking NAT
performance on Armada 3720 with Marvell 88e6190 switch.

Download speed (from internet, via Armada 3720 NAT, via switch to LAN
device) is ~750mbps and the CPU running on 100% (mostly in ksoftirq).
Upload speed is ~250mbps.

When the LAN device is connected to A3720 directly (via SFP), the
speeds are both ~1000mbps.

I realize that packing/unpacking packets with Marvell header for the
switch takes some time, but is such a performance drop expected?

This was tested with 5.0.0-rc2 and also 4.14.

Thank you.

Marek

^ permalink raw reply

* Re: [PATCH iproute2] f_flower: fix build with musl libc
From: Stephen Hemminger @ 2019-01-24 20:26 UTC (permalink / raw)
  To: Hans Dedecker; +Cc: netdev
In-Reply-To: <20190123210231.7720-1-dedeckeh@gmail.com>

On Wed, 23 Jan 2019 22:02:31 +0100
Hans Dedecker <dedeckeh@gmail.com> wrote:

> XATTR_SIZE_MAX requires the usage of linux/limits.h; let's include it
> 
> Signed-off-by: Hans Dedecker <dedeckeh@gmail.com>

Applied

^ permalink raw reply

* Re: [PATCH iproute2-next] iproute: lwtunnel set ip tunnel flags
From: Stephen Hemminger @ 2019-01-24 20:25 UTC (permalink / raw)
  To: wenxu; +Cc: dsahern, netdev
In-Reply-To: <1548338627-28538-1-git-send-email-wenxu@ucloud.cn>

On Thu, 24 Jan 2019 22:03:47 +0800
wenxu@ucloud.cn wrote:

> From: wenxu <wenxu@ucloud.cn>
> 
> ip l add dev tun type gretap external
> ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 dev gretap
> 
> For gretap example when the command set the id but don't set the
> TUNNEL_KEY flags. There is no key field in the send packet
> 
> User can set flags with key, csum, seq
> ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 key csum dev gretap
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>

Applied to iproute2

^ permalink raw reply

* Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: Marek Behun @ 2019-01-24 19:24 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Florian Fainelli, David Miller
In-Reply-To: <20190124181159.GL482@lunn.ch>

On Thu, 24 Jan 2019 19:11:59 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:
> > What properties does the cpu port node need to contain to force it?
> > phy-mode = "2500base-x"; is not enough.  
> 
> Hi Marek
> 
> For DSA ports we have:
> 
>                                                 phy-mode =
> "rgmii-txid"; fixed-link {
>                                                         speed =
> <1000>; full-duplex;
>                                                 };
> 
> See dsa_port_fixed_link_register_of()
> 
>     Andrew

Hi Andrew,
the configuration
  phy-mode = "2500base-x";
  fixed-link {
    speed = <2500>;
    full-duplex;
  };
does not work, because swphy does not support speed=2500 (only 10, 100
and 1000).
  managed = "in-band-status";
does not work either.

If I use speed = <1000>, then the swphy is created correctly, cmode is
set correctly to 2500base-x, but speed register on the port is set to
1000, and the connection does not work.

The easiest way would probably be to implement swphy to support speed
2500. But I don't know what values should the simulated PHY registers
contain...

The function dsa_port_fixed_link_register_of creates this phy device,
adjusts the link and then calls  put_device(&phydev->mdio.dev);
Does this mean that the phy device is immediately destroyed?

Marek

^ 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