Netdev List
 help / color / mirror / Atom feed
* 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

* Re: [PATCH v5 3/4] Bluetooth: Allow driver specific cmd timeout handling
From: Rajat Jain @ 2019-01-24 20:10 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
	Dmitry Torokhov, Alex Hung, Bluez mailing list,
	Linux Kernel Mailing List, linux-usb, netdev, Rajat Jain,
	Dmitry Torokhov, Raghuram Hegde, chethan.tumkur.narayan,
	Ghorai, Sukumar
In-Reply-To: <D868735A-05A4-405A-9101-8D59F4208B9E@holtmann.org>

Hi Marcel,

On Thu, Jan 24, 2019 at 1:36 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Rajat,
>
> > Add a hook to allow the BT driver to do device or command specific
> > handling in case of timeouts. This is to be used by Intel driver to
> > reset the device after certain number of timeouts.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > ---
> > v5: Drop the quirk, and rename the hook function to cmd_timeout()
> > v4: same as v1
> > v3: same as v1
> > v2: same as v1
> >
> > include/net/bluetooth/hci_core.h |  1 +
> > net/bluetooth/hci_core.c         | 10 ++++++++--
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index e5ea633ea368..624d5f2b1f36 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -437,6 +437,7 @@ struct hci_dev {
> >       int (*post_init)(struct hci_dev *hdev);
> >       int (*set_diag)(struct hci_dev *hdev, bool enable);
> >       int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> > +     void (*cmd_timeout)(struct hci_dev *hdev, struct hci_command_hdr *cmd);
> > };
> >
> > #define HCI_PHY_HANDLE(handle)        (handle & 0xff)
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 7352fe85674b..c6917f57581a 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2568,16 +2568,22 @@ static void hci_cmd_timeout(struct work_struct *work)
> > {
> >       struct hci_dev *hdev = container_of(work, struct hci_dev,
> >                                           cmd_timer.work);
> > +     struct hci_command_hdr *sent = NULL;
> >
> >       if (hdev->sent_cmd) {
> > -             struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> > -             u16 opcode = __le16_to_cpu(sent->opcode);
> > +             u16 opcode;
> > +
> > +             sent = (void *) hdev->sent_cmd->data;
> > +             opcode = __le16_to_cpu(sent->opcode);
> >
> >               bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
> >       } else {
> >               bt_dev_err(hdev, "command tx timeout");
> >       }
> >
> > +     if (hdev->cmd_timeout)
> > +             hdev->cmd_timeout(hdev, sent);
> > +
>
> drop the sent parameter. You are not using it and if at some point in the future you might want to use it, then we add it and fix up all users.

Sure, will do.

>
> And frankly, I would move the hdev->cmd_timeout call into the hdev->sent_cmd block since I do not even know if it makes sense to deal with the fallback case where hdev->sent_cmd is not available. Unless you have that kind of error case, but that is only possible if you have external injection of HCI commands.

Ummm ... my preference would be to keep it outside the block so that
the hook is always invoked in any timeout, to be consistent (whether
externally injected or not). Let me know if that is OK. I plan to send
out an iteration keeping it outside, but I'll be happy to move it in
if you feel strongly about it.

Thanks,

Rajat

>
> Regards
>
> Marcel
>

^ permalink raw reply

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

On 1/24/19 11:24 AM, Marek Behun wrote:
> 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?

Yes, we should actually migrate that code over to PHYLINK, because in
PHYLINK the fixed links don't require creating a phy_device instance.
This is something that has a potential of breaking a lot of people, so I
have not really started doing it just yet :)
-- 
Florian

^ permalink raw reply

* Re: [RFC bpf-next] bpf: allow users to opt out of releasing objects when urefs are gone
From: Jakub Kicinski @ 2019-01-24 19:28 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: alexei.starovoitov, yhs, sdf, brouer, netdev, oss-drivers
In-Reply-To: <df9b2ddd-76fb-3299-9d0e-eace4c6e9c1a@iogearbox.net>

On Thu, 24 Jan 2019 12:17:27 +0100, Daniel Borkmann wrote:
> On 01/21/2019 09:10 PM, Jakub Kicinski wrote:
> > Commit c9da161c6517 ("bpf: fix clearing on persistent program array maps")
> > added protection against dependency loops between programs and maps leading
> > to zombie objects which would never be freed.  FD maps are flushed when
> > last user reference is gone.
> > 
> > This is confusing to users of bpftool, as updates to tail call maps have
> > no effect, unless those maps are pinned (or open by another entity).  
> 
> Probably would make sense to only allow updates by providing the pinned
> map path in case of tail call map (and e.g. not by id)?

Hm, yes, or at least print a warning.

> > Today, flushing should not be a requirement for privileged user, as root
> > has access to GET_FD_BY_ID commends, and therefore can manually flush
> > the maps.  Add a flag to opt out of automatic flushing.
> > 
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> > ---
> > Hi!
> > 
> > I wonder what the reactions to this (untested) patch would be?  I don't
> > really care strongly, but perhaps others experienced a similar issue?  
> 
> My main worry with this is that this might lead to hard to debug memory
> consumption issues, for example, once apps start to make wider use of
> this flag (and even worse would load maps with BPF progs with circular
> references), then they won't be cleaned up on detach and keep holding
> potentially huge amount of kernel memory. It's true that similar issues
> would happen when long running app leaks tail call map fd or the map
> is a stale leftover node in bpf fs, but it feels there's still better
> tooling out of user space for tracing back their correlation (e.g. fds,
> file access to pinned entry) and given the fact that then there's a
> guaranteed flush once urefs are fully dropped. Not having it and scanning
> through GET_FD_BY_ID range for such cases to determine a stale tail call
> map, its potential origin, and breaking up dependencies seems much harder
> (or at least there's proper tooling missing for it today).

Makes sense, I'll toss the patch with clear conscience :)

^ permalink raw reply

* Re: [PATCH v5 4/4] Bluetooth: btusb: Use the cmd_timeout method to reset the Intel BT chip
From: Rajat Jain @ 2019-01-24 20:05 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hedberg, Greg Kroah-Hartman, David S. Miller,
	Dmitry Torokhov, Alex Hung, Bluez mailing list,
	Linux Kernel Mailing List, linux-usb, netdev, Rajat Jain,
	Dmitry Torokhov, Raghuram Hegde, chethan.tumkur.narayan,
	Ghorai, Sukumar
In-Reply-To: <7A112914-5B41-4D26-807B-021B7AC907C6@holtmann.org>

On Thu, Jan 24, 2019 at 1:46 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Rajat,
>
> > If the platform provides it, use the reset gpio to reset the Intel BT
> > chip, as part of cmd_timeout handling. This has been found helpful on
> > Intel bluetooth controllers where the firmware gets stuck and the only
> > way out is a hard reset pin provided by the platform.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > ---
> > v5: Rename the hook to cmd_timeout, and wait for 5 timeouts before
> >    resetting the device.
> > v4: Use data->flags instead of clearing the quirk in btusb_hw_reset()
> > v3: Better error handling for gpiod_get_optional()
> > v2: Handle the EPROBE_DEFER case.
> >
> > drivers/bluetooth/btusb.c | 54 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 4761499db9ee..8949ea30a1bc 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -29,6 +29,7 @@
> > #include <linux/of_device.h>
> > #include <linux/of_irq.h>
> > #include <linux/suspend.h>
> > +#include <linux/gpio/consumer.h>
> > #include <asm/unaligned.h>
> >
> > #include <net/bluetooth/bluetooth.h>
> > @@ -439,6 +440,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = {
> > #define BTUSB_BOOTING         9
> > #define BTUSB_DIAG_RUNNING    10
> > #define BTUSB_OOB_WAKE_ENABLED        11
> > +#define BTUSB_HW_RESET_ACTIVE        12
> >
> > struct btusb_data {
> >       struct hci_dev       *hdev;
> > @@ -476,6 +478,8 @@ struct btusb_data {
> >       struct usb_endpoint_descriptor *diag_tx_ep;
> >       struct usb_endpoint_descriptor *diag_rx_ep;
> >
> > +     struct gpio_desc *reset_gpio;
> > +
> >       __u8 cmdreq_type;
> >       __u8 cmdreq;
> >
> > @@ -489,8 +493,39 @@ struct btusb_data {
> >       int (*setup_on_usb)(struct hci_dev *hdev);
> >
> >       int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
> > +     unsigned num_cmd_timeout;
>
> Make this cmd_timeout_count or cmd_timeout_cnt.

Sure, will do.

>
> > };
> >
> > +
> > +static void btusb_intel_cmd_timeout(struct hci_dev *hdev,
> > +                                 struct hci_command_hdr *cmd)
> > +{
> > +     struct btusb_data *data = hci_get_drvdata(hdev);
> > +     struct gpio_desc *reset_gpio = data->reset_gpio;
> > +
> > +     bt_dev_err(hdev, "btusb num_cmd_timeout = %u", ++data->num_cmd_timeout);
>
> I would prefer if you don’t do the increase in the bt_dev_err(). And you can also scrap the “btusb “ prefix here since that is all present via bt_dev_err if really needed at some point. Actually I question the whole bt_dev_err here in the first place since the core will put our the error. You are just repeating it here.

will do.

>
> > +
> > +     if (data->num_cmd_timeout < 5)
> > +             return;
>
> So I would propose to do just this:
>
>         if (++data->cmd_timeout_count < 5)
>                 return;

will do.


>
> > +
> > +     /*
> > +      * Toggle the hard reset line if the platform provides one. The reset
> > +      * is going to yank the device off the USB and then replug. So doing
> > +      * once is enough. The cleanup is handled correctly on the way out
> > +      * (standard USB disconnect), and the new device is detected cleanly
> > +      * and bound to the driver again like it should be.
> > +      */
> > +     if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> > +             bt_dev_err(hdev, "last reset failed? Not resetting again");
> > +             return;
> > +     }
> > +
> > +     bt_dev_err(hdev, "Initiating HW reset via gpio");
> > +     gpiod_set_value(reset_gpio, 1);
> > +     mdelay(100);
> > +     gpiod_set_value(reset_gpio, 0);
> > +}
> > +
> > static inline void btusb_free_frags(struct btusb_data *data)
> > {
> >       unsigned long flags;
> > @@ -2915,6 +2950,7 @@ static int btusb_probe(struct usb_interface *intf,
> >                      const struct usb_device_id *id)
> > {
> >       struct usb_endpoint_descriptor *ep_desc;
> > +     struct gpio_desc *reset_gpio;
> >       struct btusb_data *data;
> >       struct hci_dev *hdev;
> >       unsigned ifnum_base;
> > @@ -3028,6 +3064,15 @@ static int btusb_probe(struct usb_interface *intf,
> >
> >       SET_HCIDEV_DEV(hdev, &intf->dev);
> >
> > +     reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> > +                                     GPIOD_OUT_LOW);
>
> Any reason to not use the devm_gpiod_get_optional() version here?

Yes, we're using the &data->udev->dev device (parent of the USB
interface) to fetch the gpio, so if we use the devm_* variant, the
gpio resource will not be freed immediately if the btusb_probe fails.
I'll add a comment in the code to make this more clear.

>
> > +     if (IS_ERR(reset_gpio)) {
> > +             err = PTR_ERR(reset_gpio);
> > +             goto out_free_dev;
> > +     } else if (reset_gpio) {
> > +             data->reset_gpio = reset_gpio;
> > +     }
> > +
> >       hdev->open   = btusb_open;
> >       hdev->close  = btusb_close;
> >       hdev->flush  = btusb_flush;
> > @@ -3085,6 +3130,8 @@ static int btusb_probe(struct usb_interface *intf,
> >               set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> >               set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> >               set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> > +             if (data->reset_gpio)
> > +                     hdev->cmd_timeout = btusb_intel_cmd_timeout;
> >       }
>
> Always assign hdev->cmd_timeout = btusb_intel_cmd_timeout and put it after btintel_set_bdaddr and before the quirks.

will do.

>
> Move the if (data->reset_gpio) check into btusb_intel_cmd_timeout() function and print a warning that no reset GPIO is present.

There'll be Intel platforms already out there which will obviously not
have the reset pin described in the ACPI as of today. So I'm not sure
if it is right for all of them to start complaining about a missing
"reset gpio" in case of timeout post this change. Thus I would prefer
to assign hdev->cmd_timeout only if ACPI does provide a gpio i.e.
newer platforms that want to support this feature. Thoughts?

Thanks,

Rajat

>
> >
> >       if (id->driver_info & BTUSB_INTEL_NEW) {
> > @@ -3097,6 +3144,8 @@ static int btusb_probe(struct usb_interface *intf,
> >               set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> >               set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> >               set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> > +             if (data->reset_gpio)
> > +                     hdev->cmd_timeout = btusb_intel_cmd_timeout;
> >       }
> >
> >       if (id->driver_info & BTUSB_MARVELL)
> > @@ -3226,6 +3275,8 @@ static int btusb_probe(struct usb_interface *intf,
> >       return 0;
> >
> > out_free_dev:
> > +     if (data->reset_gpio)
> > +             gpiod_put(data->reset_gpio);
> >       hci_free_dev(hdev);
> >       return err;
> > }
> > @@ -3269,6 +3320,9 @@ static void btusb_disconnect(struct usb_interface *intf)
> >       if (data->oob_wake_irq)
> >               device_init_wakeup(&data->udev->dev, false);
> >
> > +     if (data->reset_gpio)
> > +             gpiod_put(data->reset_gpio);
> > +
> >       hci_free_dev(hdev);
> > }
>
> Regards
>
> Marcel
>

^ permalink raw reply

* Re: [PATCH] PCI / ACPI: Don't clear pme_poll on device that has unreliable ACPI wake
From: Bjorn Helgaas @ 2019-01-24 20:05 UTC (permalink / raw)
  To: Kai Heng Feng
  Cc: Rafael J. Wysocki, Len Brown, jeffrey.t.kirsher, intel-wired-lan,
	netdev, linux-acpi, linux-pci, linux-kernel
In-Reply-To: <FFC6EC3F-3608-41FC-8B14-815D1747C4B6@canonical.com>

On Thu, Jan 24, 2019 at 11:29:37PM +0800, Kai Heng Feng wrote:
> > On Jan 24, 2019, at 11:15 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Wed, Jan 23, 2019 at 03:17:37PM +0800, Kai Heng Feng wrote:
> >>> On Jan 23, 2019, at 7:51 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >>> On Tue, Jan 22, 2019 at 02:45:44PM +0800, Kai-Heng Feng wrote:
> >>>> There are some e1000e devices can only be woken up from D3 one time, by
> >>>> plugging ethernet cable. Subsequent cable plugging does set PME bit
> >>>> correctly, but it still doesn't get woken up.
> >>>> 
> >>>> Since e1000e connects to the root complex directly, we rely on ACPI to
> >>>> wake it up. In this case, the GPE from _PRW only works once and stops
> >>>> working after that.
> >>>> 
> >>>> So introduce a new PCI quirk, to avoid clearing pme_poll flag for buggy
> >>>> platform firmwares that have unreliable GPE wake.
> >>> 
> >>> This quirk applies to all 0x15bb (E1000_DEV_ID_PCH_CNP_I219_LM7) and
> >>> 0x15bd (E1000_DEV_ID_PCH_CNP_I219_LM6) devices.  The e1000e driver
> >>> claims about a zillion different device IDs.
> >>> 
> >>> I would be surprised if these two devices are defective but all the
> >>> others work correctly.  Could it be that there is a problem with the
> >>> wiring on this particular motherboard or with the ACPI _PRW methods
> >>> (or the way Linux interprets them) in this firmware?
> >> 
> >> If this is a motherboard issue or platform specific, do you prefer to use
> >> DMI matches here?
> > 
> > I'm not sure what the problem is yet, so let's hold off on the exact
> > structure of the fix.
> 
> I think DMI table can put in e1000e driver instead of PCI quirk.

I don't think we should add a quirk or DMI table yet because we
haven't gotten to the root cause of this problem.  If the root cause
is a problem in the Linux code, adding a quirk will mask the problem
for this specific system, but will leave other systems with similar
problems.

> > If I understand correctly, e1000e wakeup works once, but doesn't work
> > after that.  Your lspci (from after that first wakeup, from
> > https://bugzilla.kernel.org/attachment.cgi?id=280691) shows this:
> > 
> >  00:14.0 XHC  XHCI USB
> >    Flags: PMEClk- DSI- D1- D2- ... PME(D0-,D1-,D2-,D3hot+,D3cold+)
> >    Status: D3 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-
> >  00:1f.3 HDAS audio
> >    Flags: PMEClk- DSI- D1- D2- ... PME(D0-,D1-,D2-,D3hot+,D3cold+)
> >    Status: D3 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME-
> >  00:1f.6 GLAN e1000e
> >    Flags: PMEClk- DSI+ D1- D2- ... PME(D0+,D1-,D2-,D3hot+,D3cold+)
> >    Status: D3 NoSoftRst+ PME-Enable+ DSel=0 DScale=1 PME+
> > 
> > So the e1000e PME_Status bit is still set, which means it probably
> > won't generate another PME interrupt, which would explain why wakeup
> > doesn't work.  To test this theory, can you try this:
> > 
> >  - sleep
> >  - wakeup via e1000e
> >  # DEV=00:1f.6
> >  # lspci -vvs $DEV
> >  # setpci -s $DEV CAP_PM+4.W
> >  # setpci -s $DEV CAP_PM+4.W=0x8100
> >  - sleep
> >  - attempt another wakeup via e1000e
> > 
> > If this second wakeup works, it would suggest that PME_Status isn't
> > being cleared correctly.  I see code, e.g., in
> > acpi_setup_gpe_for_wake(), that *looks* like it would arrange to clear
> > it, but I'm not very familiar with it.  Maybe there's some issue with
> > multiple devices sharing an "implicit notification" situation like
> > this.
> 
> The PME status is being cleared correctly.

I was hoping to understand this better via the experiment above, but
I'm still confused.  Here's the scenario as I understand it:

  0) fresh boot
  1) e1000e PME_Status should be 0
  2) sleep
  3) wakeup via e1000e succeeds
  4) e1000e PME_Status should be 0
  5) sleep
  6) wakeup via e1000e fails
  7) wakeup via USB succeeds
  8) e1000e PME_Status should be 0, but is actually 1

If I understand correctly, the bugzilla lspci
(https://bugzilla.kernel.org/attachment.cgi?id=280691) was collected
at point 8, and it shows PME_Status=1 when it should be 0.

If we write a 1 to PME_Status to clear it, and it remains set, that's
obviously a hardware defect, and Intel should document that in an
erratum, and a quirk would be the appropriate way to work around it.
But I doubt that's what's happening.

If e1000e changes PME_Status from 0 to 1 and we don't get an interrupt
(in this case, an SCI triggering GPE 0x6d), the problem is something
in the path outside e1000e.  Since the wakeup works the first time, we
know e1000e is *capable* of generating an interrupt, and the problem
is probably something in the way we acknowledge it.

I want to make sure we're executing the code that clears
PCI_PM_CTRL_PME_STATUS.  Would you mind applying the patch below,
turning on CONFIG_ACPI_DEBUG, booting with

  acpi.debug_layer=0x00000004 acpi.debug_level=0x00000004

and collecting the dmesg log?  I think you can also set these debug
flags via /sys/module/acpi/parameters/debug_* if that's easier.

> The lspci is captured after I plugged the ethernet cable second time,
> i.e. PME is set but not being woken up.
> 
> Kai-Heng

Bjorn

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c9d8e3c837de..91a70ffdd3a1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1927,7 +1927,7 @@ void pcie_clear_root_pme_status(struct pci_dev *dev)
 bool pci_check_pme_status(struct pci_dev *dev)
 {
 	int pmcsr_pos;
-	u16 pmcsr;
+	u16 pmcsr, pmcsr_orig;
 	bool ret = false;
 
 	if (!dev->pm_cap)
@@ -1939,6 +1939,7 @@ bool pci_check_pme_status(struct pci_dev *dev)
 		return false;
 
 	/* Clear PME status. */
+	pmcsr_orig = pmcsr;
 	pmcsr |= PCI_PM_CTRL_PME_STATUS;
 	if (pmcsr & PCI_PM_CTRL_PME_ENABLE) {
 		/* Disable PME to avoid interrupt flood. */
@@ -1948,6 +1949,9 @@ bool pci_check_pme_status(struct pci_dev *dev)
 
 	pci_write_config_word(dev, pmcsr_pos, pmcsr);
 
+	pci_read_config_word(dev, pmcsr_pos, &pmcsr);
+	pci_info(dev, "%s PM_CTRL %#06x -> %#06x\n", __func__, pmcsr_orig, pmcsr);
+
 	return ret;
 }
 
@@ -2027,18 +2031,22 @@ static void pci_pme_list_scan(struct work_struct *work)
 
 static void __pci_pme_active(struct pci_dev *dev, bool enable)
 {
-	u16 pmcsr;
+	u16 pmcsr, pmcsr_orig;
 
 	if (!dev->pme_support)
 		return;
 
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
+	pmcsr_orig = pmcsr;
 	/* Clear PME_Status by writing 1 to it and enable PME# */
 	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
 	if (!enable)
 		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
 
 	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
+
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
+	pci_info(dev, "%s PM_CTRL %#06x -> %#06x\n", __func__, pmcsr_orig, pmcsr);
 }
 
 /**

^ permalink raw reply related

* [PATCH bpf-next v2 0/3] bpf: add BPF_LWT_ENCAP_IP option to bpf_lwt_push_encap
From: Peter Oskolkov @ 2019-01-24 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev
  Cc: Peter Oskolkov, David Ahern, Willem de Bruijn, Peter Oskolkov

This patchset implements BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap
BPF helper. It enables BPF programs (specifically, BPF_PROG_TYPE_LWT_IN
and BPF_PROG_TYPE_LWT_XMIT prog types) to add IP encapsulation headers
to packets (e.g. IP/GRE, GUE, IPIP).

This is useful when thousands of different short-lived flows should be
encapped, each with different and dynamically determined destination.
Although lwtunnels can be used in some of these scenarios, the ability
to dynamically generate encap headers adds more flexibility, e.g.
when routing depends on the state of the host (reflected in global bpf
maps).

V2 changes: Added flowi-based route lookup, IPv6 encapping, and
encapping on ingress.


Peter Oskolkov (3):
  bpf: add plumbing for BPF_LWT_ENCAP_IP in bpf_lwt_push_encap
  bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap
  selftests: bpf: add test_lwt_ip_encap selftest

 include/net/lwtunnel.h                        |   3 +
 include/uapi/linux/bpf.h                      |  23 +-
 net/core/filter.c                             |  47 ++-
 net/core/lwt_bpf.c                            | 142 ++++++++
 tools/testing/selftests/bpf/Makefile          |   5 +-
 .../testing/selftests/bpf/test_lwt_ip_encap.c | 125 +++++++
 .../selftests/bpf/test_lwt_ip_encap.sh        | 316 ++++++++++++++++++
 7 files changed, 652 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_lwt_ip_encap.c
 create mode 100755 tools/testing/selftests/bpf/test_lwt_ip_encap.sh

-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply

* [PATCH bpf-next v2 2/3] bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap
From: Peter Oskolkov @ 2019-01-24 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev
  Cc: Peter Oskolkov, David Ahern, Willem de Bruijn, Peter Oskolkov
In-Reply-To: <20190124193418.81674-1-posk@google.com>

This patch implements BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap
BPF helper. It enables BPF programs (specifically, BPF_PROG_TYPE_LWT_IN
and BPF_PROG_TYPE_LWT_XMIT prog types) to add IP encapsulation headers
to packets (e.g. IP/GRE, GUE, IPIP).

This is useful when thousands of different short-lived flows should be
encapped, each with different and dynamically determined destination.
Although lwtunnels can be used in some of these scenarios, the ability
to dynamically generate encap headers adds more flexibility, e.g.
when routing depends on the state of the host (reflected in global bpf
maps).

Signed-off-by: Peter Oskolkov <posk@google.com>
---
 include/net/lwtunnel.h |   3 +
 net/core/filter.c      |   3 +-
 net/core/lwt_bpf.c     | 142 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 33fd9ba7e0e5..f0973eca8036 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -126,6 +126,8 @@ int lwtunnel_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b);
 int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb);
 int lwtunnel_input(struct sk_buff *skb);
 int lwtunnel_xmit(struct sk_buff *skb);
+int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len,
+			  bool ingress);
 
 static inline void lwtunnel_set_redirect(struct dst_entry *dst)
 {
@@ -138,6 +140,7 @@ static inline void lwtunnel_set_redirect(struct dst_entry *dst)
 		dst->input = lwtunnel_input;
 	}
 }
+
 #else
 
 static inline void lwtstate_free(struct lwtunnel_state *lws)
diff --git a/net/core/filter.c b/net/core/filter.c
index fd3ae092d3d7..81d18660c38b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -73,6 +73,7 @@
 #include <linux/seg6_local.h>
 #include <net/seg6.h>
 #include <net/seg6_local.h>
+#include <net/lwtunnel.h>
 
 /**
  *	sk_filter_trim_cap - run a packet through a socket filter
@@ -4796,7 +4797,7 @@ static int bpf_push_seg6_encap(struct sk_buff *skb, u32 type, void *hdr, u32 len
 static int bpf_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len,
 			     bool ingress)
 {
-	return -EINVAL;  /* Implemented in the next patch. */
+	return bpf_lwt_push_ip_encap(skb, hdr, len, ingress);
 }
 
 BPF_CALL_4(bpf_lwt_in_push_encap, struct sk_buff *, skb, u32, type, void *, hdr,
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index 3e85437f7106..a3f79bff3776 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -16,6 +16,7 @@
 #include <linux/types.h>
 #include <linux/bpf.h>
 #include <net/lwtunnel.h>
+#include <net/ip6_route.h>
 
 struct bpf_lwt_prog {
 	struct bpf_prog *prog;
@@ -55,6 +56,7 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
 
 	switch (ret) {
 	case BPF_OK:
+	case BPF_LWT_REROUTE:
 		break;
 
 	case BPF_REDIRECT:
@@ -97,6 +99,8 @@ static int bpf_input(struct sk_buff *skb)
 		ret = run_lwt_bpf(skb, &bpf->in, dst, NO_REDIRECT);
 		if (ret < 0)
 			return ret;
+		if (ret == BPF_LWT_REROUTE)
+			return dst_input(skb);
 	}
 
 	if (unlikely(!dst->lwtstate->orig_input)) {
@@ -168,6 +172,13 @@ static int bpf_xmit(struct sk_buff *skb)
 			return LWTUNNEL_XMIT_CONTINUE;
 		case BPF_REDIRECT:
 			return LWTUNNEL_XMIT_DONE;
+		case BPF_LWT_REROUTE:
+			ret = dst_output(dev_net(skb_dst(skb)->dev),
+					 skb->sk, skb);
+			if (unlikely(ret))
+				return ret;
+			/* ip[6]_finish_output2 understand LWTUNNEL_XMIT_DONE */
+			return LWTUNNEL_XMIT_DONE;
 		default:
 			return ret;
 		}
@@ -389,6 +400,137 @@ static const struct lwtunnel_encap_ops bpf_encap_ops = {
 	.owner		= THIS_MODULE,
 };
 
+int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
+{
+	struct dst_entry *dst = NULL;
+	struct iphdr *iph;
+	bool ipv4;
+	int err;
+
+	if (unlikely(len < sizeof(struct iphdr) || len > LWT_BPF_MAX_HEADROOM))
+		return -EINVAL;
+
+	/* validate protocol and length */
+	iph = (struct iphdr *)hdr;
+	if (iph->version == 4) {
+		ipv4 = true;
+		if (iph->ihl * 4 > len)
+			return -EINVAL;
+	} else if (iph->version == 6) {
+		ipv4 = false;
+		if (unlikely(len < sizeof(struct ipv6hdr)))
+			return -EINVAL;
+	} else {
+		return -EINVAL;
+	}
+
+	/* allocate enough space for the encap headers + L2 hdr */
+	if (ingress) {
+		err = skb_cow_head(skb, len + skb->mac_len);
+		if (unlikely(err))
+			return err;
+	} else {
+		/* ip_route_input_noref below does route lookup and dst
+		 * drop/set for ingress. There is no similar function for
+		 * egress, so we need to do route lookup and replace skb's
+		 * dst in this function.
+		 */
+		struct sock *sk;
+		struct net *net;
+
+		sk = sk_to_full_sk(skb->sk);
+		if (sk)
+			net = sock_net(sk);
+		else
+			net = dev_net(skb_dst(skb)->dev);
+
+		if (ipv4) {
+			struct flowi4 fl4 = {0};
+			struct rtable *rt;
+
+			fl4.flowi4_oif = sk ? sk->sk_bound_dev_if : 0;
+			fl4.flowi4_mark = skb->mark;
+			fl4.flowi4_uid = sock_net_uid(net, sk);
+			fl4.flowi4_tos = RT_TOS(iph->tos);
+			fl4.flowi4_flags = FLOWI_FLAG_ANYSRC;
+			fl4.flowi4_proto = iph->protocol;
+			fl4.daddr = iph->daddr;
+			fl4.saddr = iph->saddr;
+
+			rt = ip_route_output_key(net, &fl4);
+			if (IS_ERR(rt) || rt->dst.error)
+				return -EINVAL;
+			dst = &rt->dst;
+		} else {
+			struct ipv6hdr *iph6 = (struct ipv6hdr *)hdr;
+			struct flowi6 fl6 = {0};
+
+			fl6.flowi6_oif = sk ? sk->sk_bound_dev_if : 0;
+			fl6.flowi6_mark = skb->mark;
+			fl6.flowi6_uid = sock_net_uid(net, sk);
+			fl6.flowlabel = ip6_flowinfo(iph6);
+			fl6.flowi6_proto = iph6->nexthdr;
+			fl6.daddr = iph6->daddr;
+			fl6.saddr = iph6->saddr;
+
+			dst = ip6_route_output(net, skb->sk, &fl6);
+			if (IS_ERR(dst) || dst->error)
+				return -EINVAL;
+		}
+
+		err = skb_cow_head(skb, len + LL_RESERVED_SPACE(dst->dev));
+		if (unlikely(err))
+			return err;
+	}
+
+	/* push the encap headers and fix pointers */
+	skb_reset_inner_headers(skb);
+	skb->encapsulation = 1;
+	skb_push(skb, len);
+	if (ingress)
+		skb_postpush_rcsum(skb, iph, len);
+	skb_reset_network_header(skb);
+	iph = ip_hdr(skb);
+	memcpy(iph, hdr, len);
+	bpf_compute_data_pointers(skb);
+
+	/* final skb touches + routing */
+	if (ipv4) {
+		skb->protocol = htons(ETH_P_IP);
+		if (iph->ihl * 4 < len)
+			skb_set_transport_header(skb, iph->ihl * 4);
+
+		if (!iph->check)
+			iph->check = ip_fast_csum((unsigned char *)iph,
+						  iph->ihl);
+
+		if (ingress) {
+			err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
+						   iph->tos, skb_dst(skb)->dev);
+			if (err)
+				return err;
+		} else {
+			skb_dst_drop(skb);
+			skb_dst_set(skb, dst);
+		}
+	} else {
+		skb->protocol = htons(ETH_P_IPV6);
+		if (sizeof(struct ipv6hdr) < len)
+			skb_set_transport_header(skb, sizeof(struct ipv6hdr));
+
+		if (ingress) {
+			ip6_route_input(skb);
+			if (skb_dst(skb)->error)
+				return skb_dst(skb)->error;
+		} else {
+			skb_dst_drop(skb);
+			skb_dst_set(skb, dst);
+		}
+	}
+
+	return 0;
+}
+
 static int __init bpf_lwt_init(void)
 {
 	return lwtunnel_encap_add_ops(&bpf_encap_ops, LWTUNNEL_ENCAP_BPF);
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* [PATCH bpf-next v2 3/3] selftests: bpf: add test_lwt_ip_encap selftest
From: Peter Oskolkov @ 2019-01-24 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev
  Cc: Peter Oskolkov, David Ahern, Willem de Bruijn, Peter Oskolkov
In-Reply-To: <20190124193418.81674-1-posk@google.com>

This patch adds a bpf self-test to cover BPF_LWT_ENCAP_IP mode
in bpf_lwt_push_encap.

Covered:
- encapping in LWT_IN and LWT_XMIT
- IPv4 and IPv6

Signed-off-by: Peter Oskolkov <posk@google.com>
---
 tools/testing/selftests/bpf/Makefile          |   5 +-
 .../testing/selftests/bpf/test_lwt_ip_encap.c | 125 +++++++
 .../selftests/bpf/test_lwt_ip_encap.sh        | 316 ++++++++++++++++++
 3 files changed, 444 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_lwt_ip_encap.c
 create mode 100755 tools/testing/selftests/bpf/test_lwt_ip_encap.sh

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 70229de510f5..407c51cc5f07 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -39,7 +39,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
 	get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
 	test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o \
 	test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o test_stack_map.o \
-	xdp_dummy.o test_map_in_map.o
+	xdp_dummy.o test_map_in_map.o test_lwt_ip_encap.o
 
 # Order correspond to 'make run_tests' order
 TEST_PROGS := test_kmod.sh \
@@ -53,7 +53,8 @@ TEST_PROGS := test_kmod.sh \
 	test_lirc_mode2.sh \
 	test_skb_cgroup_id.sh \
 	test_flow_dissector.sh \
-	test_xdp_vlan.sh
+	test_xdp_vlan.sh \
+	test_lwt_ip_encap.sh
 
 TEST_PROGS_EXTENDED := with_addr.sh \
 	tcp_client.py \
diff --git a/tools/testing/selftests/bpf/test_lwt_ip_encap.c b/tools/testing/selftests/bpf/test_lwt_ip_encap.c
new file mode 100644
index 000000000000..a7014277f3fe
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_lwt_ip_encap.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <string.h>
+#include "bpf_helpers.h"
+#include "bpf_endian.h"
+
+#define BPF_LWT_ENCAP_IP 2
+#define BPF_LWT_REROUTE 128
+
+struct iphdr {
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	__u8	ihl:4,
+		version:4;
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+	__u8	version:4,
+		ihl:4;
+#else
+#error "Fix your compiler's __BYTE_ORDER__?!"
+#endif
+	__u8	tos;
+	__be16	tot_len;
+	__be16	id;
+	__be16	frag_off;
+	__u8	ttl;
+	__u8	protocol;
+	__sum16	check;
+	__be32	saddr;
+	__be32	daddr;
+};
+
+struct ipv6hdr {
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	__u8	priority:4,
+		version:4;
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+	__u8	version:4,
+		priority:4;
+#else
+#error "Fix your compiler's __BYTE_ORDER__?!"
+#endif
+	__u8	flow_lbl[3];
+
+	__be16	payload_len;
+	__u8	nexthdr;
+	__u8	hop_limit;
+
+	__u8	saddr[16];
+	__u8	daddr[16];
+};
+
+struct grehdr {
+	__be16 flags;
+	__be16 protocol;
+};
+
+SEC("encap_gre")
+int bpf_lwt_encap_gre(struct __sk_buff *skb)
+{
+	struct encap_hdr {
+		struct iphdr iph;
+		struct grehdr greh;
+	} hdr;
+	int err;
+
+	memset(&hdr, 0, sizeof(struct encap_hdr));
+
+	hdr.iph.ihl = 5;
+	hdr.iph.version = 4;
+	hdr.iph.ttl = 0x40;
+	hdr.iph.protocol = 47;  /* IPPROTO_GRE */
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	hdr.iph.saddr = 0x640110ac;  /* 172.16.1.100 */
+	hdr.iph.daddr = 0x641010ac;  /* 172.16.16.100 */
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+	hdr.iph.saddr = 0xac100164;  /* 172.16.1.100 */
+	hdr.iph.daddr = 0xac101064;  /* 172.16.16.100 */
+#else
+#error "Fix your compiler's __BYTE_ORDER__?!"
+#endif
+	hdr.iph.tot_len = bpf_htons(skb->len + sizeof(struct encap_hdr));
+
+	hdr.greh.protocol = skb->protocol;
+
+	err = bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &hdr,
+				 sizeof(struct encap_hdr));
+	if (err)
+		return BPF_DROP;
+	return BPF_LWT_REROUTE;
+}
+
+SEC("encap_gre6")
+int bpf_lwt_encap_gre6(struct __sk_buff *skb)
+{
+	struct encap_hdr {
+		struct ipv6hdr ip6hdr;
+		struct grehdr greh;
+	} hdr;
+	int err;
+
+	memset(&hdr, 0, sizeof(struct encap_hdr));
+
+	hdr.ip6hdr.version = 6;
+	hdr.ip6hdr.payload_len = bpf_htons(skb->len + sizeof(struct grehdr));
+	hdr.ip6hdr.nexthdr = 47;  /* IPPROTO_GRE */
+	hdr.ip6hdr.hop_limit = 0x40;
+	/* fb01::1 */
+	hdr.ip6hdr.saddr[0] = 0xfb;
+	hdr.ip6hdr.saddr[1] = 1;
+	hdr.ip6hdr.saddr[15] = 1;
+	/* fb10::1 */
+	hdr.ip6hdr.daddr[0] = 0xfb;
+	hdr.ip6hdr.daddr[1] = 0x10;
+	hdr.ip6hdr.daddr[15] = 1;
+
+	hdr.greh.protocol = skb->protocol;
+
+	err = bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &hdr,
+				 sizeof(struct encap_hdr));
+	if (err)
+		return BPF_DROP;
+
+	return BPF_LWT_REROUTE;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_lwt_ip_encap.sh b/tools/testing/selftests/bpf/test_lwt_ip_encap.sh
new file mode 100755
index 000000000000..4f511587bb74
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_lwt_ip_encap.sh
@@ -0,0 +1,316 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Setup/topology:
+#
+#    NS1             NS2             NS3
+#   veth1 <---> veth2   veth3 <---> veth4 (the top route)
+#   veth5 <---> veth6   veth7 <---> veth8 (the bottom route)
+#
+#   each vethN gets IPv[4|6]_N address
+#
+#   IPv*_SRC = IPv*_1
+#   IPv*_DST = IPv*_4
+#
+#   all tests test pings from IPv*_SRC to IPv*_DST
+#
+#   by default, routes are configured to allow packets to go
+#   IP*_1 <=> IP*_2 <=> IP*_3 <=> IP*_4 (the top route)
+#
+#   a GRE device is installed in NS3 with IPv*_GRE, and
+#   NS1/NS2 are configured to route packets to IPv*_GRE via IP*_8
+#   (the bottom route)
+#
+# Tests:
+#
+#   1. routes NS2->IPv*_DST are brought down, so the only way a ping
+#      from IP*_SRC to IP*_DST can work is via IPv*_GRE
+#
+#   2a. in an egress test, a bpf LWT_XMIT program is installed on veth1
+#       that encaps the packets with an IP/GRE header to route to IPv*_GRE
+#
+#       ping: SRC->[encap at veth1:egress]->GRE:decap->DST
+#       ping replies go DST->SRC directly
+#
+#   2b. in an ingress test, a bpf LWT_IN program is installed on veth2
+#       that encaps the packets with an IP/GRE header to route to IPv*_GRE
+#
+#       ping: SRC->[encap at veth2:ingress]->GRE:decap->DST
+#       ping replies go DST->SRC directly
+
+set +x  # debug OFF
+set -e  # exit on error
+
+if [[ $EUID -ne 0 ]]; then
+	echo "This script must be run as root"
+	echo "FAIL"
+	exit 1
+fi
+
+readonly NS1="ns1-$(mktemp -u XXXXXX)"
+readonly NS2="ns2-$(mktemp -u XXXXXX)"
+readonly NS3="ns3-$(mktemp -u XXXXXX)"
+
+readonly IPv4_1="172.16.1.100"
+readonly IPv4_2="172.16.2.100"
+readonly IPv4_3="172.16.3.100"
+readonly IPv4_4="172.16.4.100"
+readonly IPv4_5="172.16.5.100"
+readonly IPv4_6="172.16.6.100"
+readonly IPv4_7="172.16.7.100"
+readonly IPv4_8="172.16.8.100"
+readonly IPv4_GRE="172.16.16.100"
+
+readonly IPv4_SRC=$IPv4_1
+readonly IPv4_DST=$IPv4_4
+
+readonly IPv6_1="fb01::1"
+readonly IPv6_2="fb02::1"
+readonly IPv6_3="fb03::1"
+readonly IPv6_4="fb04::1"
+readonly IPv6_5="fb05::1"
+readonly IPv6_6="fb06::1"
+readonly IPv6_7="fb07::1"
+readonly IPv6_8="fb08::1"
+readonly IPv6_GRE="fb10::1"
+
+readonly IPv6_SRC=$IPv6_1
+readonly IPv6_DST=$IPv6_4
+
+setup() {
+set -e  # exit on error
+	# create devices and namespaces
+	ip netns add "${NS1}"
+	ip netns add "${NS2}"
+	ip netns add "${NS3}"
+
+	ip link add veth1 type veth peer name veth2
+	ip link add veth3 type veth peer name veth4
+	ip link add veth5 type veth peer name veth6
+	ip link add veth7 type veth peer name veth8
+
+	ip netns exec ${NS2} sysctl -w net.ipv4.ip_forward=1 2>&1 > /dev/null
+	ip netns exec ${NS2} sysctl -w net.ipv6.conf.all.forwarding=1 2>&1 > /dev/null
+
+	ip link set veth1 netns ${NS1}
+	ip link set veth2 netns ${NS2}
+	ip link set veth3 netns ${NS2}
+	ip link set veth4 netns ${NS3}
+	ip link set veth5 netns ${NS1}
+	ip link set veth6 netns ${NS2}
+	ip link set veth7 netns ${NS2}
+	ip link set veth8 netns ${NS3}
+
+	# configure addesses: the top route (1-2-3-4)
+	ip -netns ${NS1}    addr add ${IPv4_1}/24  dev veth1
+	ip -netns ${NS2}    addr add ${IPv4_2}/24  dev veth2
+	ip -netns ${NS2}    addr add ${IPv4_3}/24  dev veth3
+	ip -netns ${NS3}    addr add ${IPv4_4}/24  dev veth4
+	ip -netns ${NS1} -6 addr add ${IPv6_1}/128 dev veth1
+	ip -netns ${NS2} -6 addr add ${IPv6_2}/128 dev veth2
+	ip -netns ${NS2} -6 addr add ${IPv6_3}/128 dev veth3
+	ip -netns ${NS3} -6 addr add ${IPv6_4}/128 dev veth4
+
+	# configure addresses: the bottom route (5-6-7-8)
+	ip -netns ${NS1}    addr add ${IPv4_5}/24  dev veth5
+	ip -netns ${NS2}    addr add ${IPv4_6}/24  dev veth6
+	ip -netns ${NS2}    addr add ${IPv4_7}/24  dev veth7
+	ip -netns ${NS3}    addr add ${IPv4_8}/24  dev veth8
+	ip -netns ${NS1} -6 addr add ${IPv6_5}/128 dev veth5
+	ip -netns ${NS2} -6 addr add ${IPv6_6}/128 dev veth6
+	ip -netns ${NS2} -6 addr add ${IPv6_7}/128 dev veth7
+	ip -netns ${NS3} -6 addr add ${IPv6_8}/128 dev veth8
+
+
+	ip -netns ${NS1} link set dev veth1 up
+	ip -netns ${NS2} link set dev veth2 up
+	ip -netns ${NS2} link set dev veth3 up
+	ip -netns ${NS3} link set dev veth4 up
+	ip -netns ${NS1} link set dev veth5 up
+	ip -netns ${NS2} link set dev veth6 up
+	ip -netns ${NS2} link set dev veth7 up
+	ip -netns ${NS3} link set dev veth8 up
+
+	# configure routes: IP*_SRC -> veth1/IP*_2 (= top route) default;
+	# the bottom route to specific bottom addresses
+
+	# NS1
+	# top route
+	ip -netns ${NS1}    route add ${IPv4_2}/32  dev veth1
+	ip -netns ${NS1}    route add default dev veth1 via ${IPv4_2}  # go top by default
+	ip -netns ${NS1} -6 route add ${IPv6_2}/128 dev veth1
+	ip -netns ${NS1} -6 route add default dev veth1 via ${IPv6_2}  # go top by default
+	# bottom route
+	ip -netns ${NS1}    route add ${IPv4_6}/32  dev veth5
+	ip -netns ${NS1}    route add ${IPv4_7}/32  dev veth5 via ${IPv4_6}
+	ip -netns ${NS1}    route add ${IPv4_8}/32  dev veth5 via ${IPv4_6}
+	ip -netns ${NS1} -6 route add ${IPv6_6}/128 dev veth5
+	ip -netns ${NS1} -6 route add ${IPv6_7}/128 dev veth5 via ${IPv6_6}
+	ip -netns ${NS1} -6 route add ${IPv6_8}/128 dev veth5 via ${IPv6_6}
+
+	# NS2
+	# top route
+	ip -netns ${NS2}    route add ${IPv4_1}/32  dev veth2
+	ip -netns ${NS2}    route add ${IPv4_4}/32  dev veth3
+	ip -netns ${NS2} -6 route add ${IPv6_1}/128 dev veth2
+	ip -netns ${NS2} -6 route add ${IPv6_4}/128 dev veth3
+	# bottom route
+	ip -netns ${NS2}    route add ${IPv4_5}/32  dev veth6
+	ip -netns ${NS2}    route add ${IPv4_8}/32  dev veth7
+	ip -netns ${NS2} -6 route add ${IPv6_5}/128 dev veth6
+	ip -netns ${NS2} -6 route add ${IPv6_8}/128 dev veth7
+
+	# NS3
+	# top route
+	ip -netns ${NS3}    route add ${IPv4_3}/32  dev veth4
+	ip -netns ${NS3}    route add ${IPv4_1}/32  dev veth4 via ${IPv4_3}
+	ip -netns ${NS3}    route add ${IPv4_2}/32  dev veth4 via ${IPv4_3}
+	ip -netns ${NS3} -6 route add ${IPv6_3}/128 dev veth4
+	ip -netns ${NS3} -6 route add ${IPv6_1}/128 dev veth4 via ${IPv6_3}
+	ip -netns ${NS3} -6 route add ${IPv6_2}/128 dev veth4 via ${IPv6_3}
+	# bottom route
+	ip -netns ${NS3}    route add ${IPv4_7}/32  dev veth8
+	ip -netns ${NS3}    route add ${IPv4_5}/32  dev veth8 via ${IPv4_7}
+	ip -netns ${NS3}    route add ${IPv4_6}/32  dev veth8 via ${IPv4_7}
+	ip -netns ${NS3} -6 route add ${IPv6_7}/128 dev veth8
+	ip -netns ${NS3} -6 route add ${IPv6_5}/128 dev veth8 via ${IPv6_7}
+	ip -netns ${NS3} -6 route add ${IPv6_6}/128 dev veth8 via ${IPv6_7}
+
+	# configure IPv4 GRE device in NS3, and a route to it via the "bottom" route
+	ip -netns ${NS3} tunnel add gre_dev mode gre remote ${IPv4_1} local ${IPv4_GRE} ttl 255
+	ip -netns ${NS3} link set gre_dev up
+	ip -netns ${NS3} addr add ${IPv4_GRE} dev gre_dev
+	ip -netns ${NS1} route add ${IPv4_GRE}/32 dev veth5 via ${IPv4_6}
+	ip -netns ${NS2} route add ${IPv4_GRE}/32 dev veth7 via ${IPv4_8}
+
+
+	# configure IPv6 GRE device in NS3, and a route to it via the "bottom" route
+	ip -netns ${NS3} -6 tunnel add name gre6_dev mode ip6gre remote ${IPv6_1} local ${IPv6_GRE} ttl 255
+	ip -netns ${NS3} link set gre6_dev up
+	ip -netns ${NS3} -6 addr add ${IPv6_GRE} dev gre6_dev
+	ip -netns ${NS1} -6 route add ${IPv6_GRE}/128 dev veth5 via ${IPv6_6}
+	ip -netns ${NS2} -6 route add ${IPv6_GRE}/128 dev veth7 via ${IPv6_8}
+
+	# rp_filter gets confused by what these tests are doing, so disable it
+	ip netns exec ${NS1} sysctl -w net.ipv4.conf.all.rp_filter=0 2>&1 > /dev/null
+	ip netns exec ${NS2} sysctl -w net.ipv4.conf.all.rp_filter=0 2>&1 > /dev/null
+	ip netns exec ${NS3} sysctl -w net.ipv4.conf.all.rp_filter=0 2>&1 > /dev/null
+}
+
+cleanup() {
+	ip netns del ${NS1} 2> /dev/null
+	ip netns del ${NS2} 2> /dev/null
+	ip netns del ${NS3} 2> /dev/null
+}
+
+trap cleanup EXIT
+
+test_ping() {
+	local readonly PROTO=$1
+	local readonly EXPECTED=$2
+	local RET=0
+
+	set +e
+	if [ "${PROTO}" == "IPv4" ] ; then
+		ip netns exec ${NS1} ping  -c 1 -W 1 -I ${IPv4_SRC} ${IPv4_DST} 2>&1 > /dev/null
+		# ip netns exec ${NS1} ping  -c 1 -W 10 -I ${IPv4_SRC} ${IPv4_DST}
+		RET=$?
+	elif [ "${PROTO}" == "IPv6" ] ; then
+		ip netns exec ${NS1} ping6 -c 1 -W 6 -I ${IPv6_SRC} ${IPv6_DST} 2>&1 > /dev/null
+		# ip netns exec ${NS1} ping6 -c 1 -W 6 -I ${IPv6_SRC} ${IPv6_DST}
+		RET=$?
+	else
+		echo "test_ping: unknown PROTO: ${PROTO}"
+		exit 1
+	fi
+	set -e
+
+	if [ "0" != "${RET}" ]; then
+		RET=1
+	fi
+
+	if [ "${EXPECTED}" != "${RET}" ] ; then
+		echo "FAIL: test_ping: ${RET}"
+		exit 1
+	fi
+}
+
+test_egress() {
+	local readonly ENCAP=$1
+	echo "starting egress ${ENCAP} encap test"
+	setup
+
+	# need to wait a bit for IPv6 to autoconf, otherwise
+	# ping6 sometimes fails with "unable to bind to address"
+	sleep 1
+
+	# by default, pings work
+	test_ping IPv4 0
+	test_ping IPv6 0
+
+	# remove NS2->DST routes, ping fails
+	ip -netns ${NS2}    route del ${IPv4_DST}/32  dev veth3
+	ip -netns ${NS2} -6 route del ${IPv6_DST}/128 dev veth3
+	test_ping IPv4 1
+	test_ping IPv6 1
+
+	# install replacement routes (LWT/eBPF), pings succeed
+	if [ "${ENCAP}" == "IPv4" ] ; then
+		ip -netns ${NS1} route add ${IPv4_DST} encap bpf xmit obj test_lwt_ip_encap.o sec encap_gre dev veth1
+		ip -netns ${NS1} -6 route add ${IPv6_DST} encap bpf xmit obj test_lwt_ip_encap.o sec encap_gre dev veth1
+	elif [ "${ENCAP}" == "IPv6" ] ; then
+		ip -netns ${NS1} route add ${IPv4_DST} encap bpf xmit obj test_lwt_ip_encap.o sec encap_gre6 dev veth1
+		ip -netns ${NS1} -6 route add ${IPv6_DST} encap bpf xmit obj test_lwt_ip_encap.o sec encap_gre6 dev veth1
+	else
+		echo "FAIL: unknown encap ${ENCAP}"
+	fi
+	test_ping IPv4 0
+	test_ping IPv6 0
+
+	cleanup
+	echo "PASS"
+}
+
+test_ingress() {
+	local readonly ENCAP=$1
+	echo "starting ingress ${ENCAP} encap test"
+	setup
+
+	# need to wait a bit for IPv6 to autoconf, otherwise
+	# ping6 sometimes fails with "unable to bind to address"
+	sleep 1
+
+	# by default, pings work
+	test_ping IPv4 0
+	test_ping IPv6 0
+
+	# remove NS2->DST routes, pings fail
+	ip -netns ${NS2}    route del ${IPv4_DST}/32  dev veth3
+	ip -netns ${NS2} -6 route del ${IPv6_DST}/128 dev veth3
+	test_ping IPv4 1
+	test_ping IPv6 1
+
+	# install replacement routes (LWT/eBPF), pings succeed
+	if [ "${ENCAP}" == "IPv4" ] ; then
+		ip -netns ${NS2} route add ${IPv4_DST} encap bpf in obj test_lwt_ip_encap.o sec encap_gre dev veth2
+		ip -netns ${NS2} -6 route add ${IPv6_DST} encap bpf in obj test_lwt_ip_encap.o sec encap_gre dev veth2
+	elif [ "${ENCAP}" == "IPv6" ] ; then
+		ip -netns ${NS2} route add ${IPv4_DST} encap bpf in obj test_lwt_ip_encap.o sec encap_gre6 dev veth2
+		ip -netns ${NS2} -6 route add ${IPv6_DST} encap bpf in obj test_lwt_ip_encap.o sec encap_gre6 dev veth2
+	else
+		echo "FAIL: unknown encap ${ENCAP}"
+	fi
+	test_ping IPv4 0
+	test_ping IPv6 0
+
+	cleanup
+	echo "PASS"
+}
+
+test_egress IPv4
+test_egress IPv6
+
+test_ingress IPv4
+test_ingress IPv6
+
+echo "all tests passed"
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

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

On 1/24/19 11:37 AM, Marek Behun wrote:
> On Thu, 24 Jan 2019 11:27:54 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 1/24/19 11:24 AM, Marek Behun wrote:
>>> 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?  
>>
>> Yes, we should actually migrate that code over to PHYLINK, because in
>> PHYLINK the fixed links don't require creating a phy_device instance.
>> This is something that has a potential of breaking a lot of people,
>> so I have not really started doing it just yet :)
> 
> phylink_create requires a net_device as first argument. what should be
> the device for cpu/dsa ports?

We would have to somehow change that assumption or introduce some low
level function for PHYLINK that are just meant to deal with the phylink
object, without the netdev, that is among other things why it has not
happened yet.
-- 
Florian

^ 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:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, netdev, David Miller
In-Reply-To: <3c81ed78-4f0e-37b1-710c-6d96c3e76577@gmail.com>

On Thu, 24 Jan 2019 11:27:54 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 11:24 AM, Marek Behun wrote:
> > 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?  
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people,
> so I have not really started doing it just yet :)

phylink_create requires a net_device as first argument. what should be
the device for cpu/dsa ports?

Marek

^ 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:39 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, netdev, David Miller
In-Reply-To: <3c81ed78-4f0e-37b1-710c-6d96c3e76577@gmail.com>

On Thu, 24 Jan 2019 11:27:54 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 11:24 AM, Marek Behun wrote:
> > 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?  
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people,
> so I have not really started doing it just yet :)

Can't then this patch be applied so that Turris Mox will work again? At
least till the cpu/dsa port connection is converted to phylink.
Marek

^ permalink raw reply

* [PATCH 4.20 010/127] netlink: fixup regression in RTM_GETADDR
From: Greg Kroah-Hartman @ 2019-01-24 19:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, David Ahern, David S . Miller, netdev,
	Arthur Gautier
In-Reply-To: <20190124190211.984305387@linuxfoundation.org>

4.20-stable review patch.  If anyone has any objections, please let me know.

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

From: Arthur Gautier <baloo@gandi.net>

[ Upstream commit 7c1e8a3817c55d73b27cc29b84075999c8894179 ]

This commit fixes a regression in AF_INET/RTM_GETADDR and
AF_INET6/RTM_GETADDR.

Before this commit, the kernel would stop dumping addresses once the first
skb was full and end the stream with NLMSG_DONE(-EMSGSIZE). The error
shouldn't be sent back to netlink_dump so the callback is kept alive. The
userspace is expected to call back with a new empty skb.

Changes from V1:
 - The error is not handled in netlink_dump anymore but rather in
   inet_dump_ifaddr and inet6_dump_addr directly as suggested by
   David Ahern.

Fixes: d7e38611b81e ("net/ipv4: Put target net when address dump fails due to bad attributes")
Fixes: 242afaa6968c ("net/ipv6: Put target net when address dump fails due to bad attributes")

Cc: David Ahern <dsahern@gmail.com>
Cc: "David S . Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Arthur Gautier <baloo@gandi.net>
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/devinet.c  |    2 +-
 net/ipv6/addrconf.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1826,7 +1826,7 @@ put_tgt_net:
 	if (fillargs.netnsid >= 0)
 		put_net(tgt_net);
 
-	return err < 0 ? err : skb->len;
+	return skb->len ? : err;
 }
 
 static void rtmsg_ifa(int event, struct in_ifaddr *ifa, struct nlmsghdr *nlh,
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5154,7 +5154,7 @@ put_tgt_net:
 	if (fillargs.netnsid >= 0)
 		put_net(tgt_net);
 
-	return err < 0 ? err : skb->len;
+	return skb->len ? : err;
 }
 
 static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)



^ permalink raw reply

* [PATCH bpf-next v2 1/3] bpf: add plumbing for BPF_LWT_ENCAP_IP in bpf_lwt_push_encap
From: Peter Oskolkov @ 2019-01-24 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev
  Cc: Peter Oskolkov, David Ahern, Willem de Bruijn, Peter Oskolkov
In-Reply-To: <20190124193418.81674-1-posk@google.com>

This patch adds all needed plumbing in preparation to allowing
bpf programs to do IP encapping via bpf_lwt_push_encap. Actual
implementation is added in the next patch in the patchset.

Of note:
- bpf_lwt_push_encap can now be called from BPF_PROG_TYPE_LWT_XMIT
  prog types in addition to BPF_PROG_TYPE_LWT_IN;
- as route lookups are different for ingress vs egress, the single
  external bpf_lwt_push_encap BPF helper is routed internally to
  either bpf_lwt_in_push_encap or bpf_lwt_xmit_push_encap BPF_CALLs,
  depending on prog type.

Signed-off-by: Peter Oskolkov <posk@google.com>
---
 include/uapi/linux/bpf.h | 23 ++++++++++++++++++--
 net/core/filter.c        | 46 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 2940a9854f6d..d4852feb14e4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2014,6 +2014,16 @@ union bpf_attr {
  *			Only works if *skb* contains an IPv6 packet. Insert a
  *			Segment Routing Header (**struct ipv6_sr_hdr**) inside
  *			the IPv6 header.
+ *		**BPF_LWT_ENCAP_IP**
+ *			IP encapsulation (GRE/GUE/IPIP/etc). The outer header
+ *			must be IPv4 or IPv6, followed by zero or more
+ *			additional headers, up to LWT_BPF_MAX_HEADROOM total
+ *			bytes in all prepended headers.
+ *
+ *		BPF_LWT_ENCAP_SEG6*** types can be called by bpf programs of
+ *		type BPF_PROG_TYPE_LWT_IN; BPF_LWT_ENCAP_IP type can be called
+ *		by bpf programs of types BPF_PROG_TYPE_LWT_IN and
+ *		BPF_PROG_TYPE_LWT_XMIT.
  *
  * 		A call to this helper is susceptible to change the underlaying
  * 		packet buffer. Therefore, at load time, all checks on pointers
@@ -2494,7 +2504,8 @@ enum bpf_hdr_start_off {
 /* Encapsulation type for BPF_FUNC_lwt_push_encap helper. */
 enum bpf_lwt_encap_mode {
 	BPF_LWT_ENCAP_SEG6,
-	BPF_LWT_ENCAP_SEG6_INLINE
+	BPF_LWT_ENCAP_SEG6_INLINE,
+	BPF_LWT_ENCAP_IP,
 };
 
 #define __bpf_md_ptr(type, name)	\
@@ -2582,7 +2593,15 @@ enum bpf_ret_code {
 	BPF_DROP = 2,
 	/* 3-6 reserved */
 	BPF_REDIRECT = 7,
-	/* >127 are reserved for prog type specific return codes */
+	/* >127 are reserved for prog type specific return codes.
+	 *
+	 * BPF_LWT_REROUTE: used by BPF_PROG_TYPE_LWT_IN and
+	 *    BPF_PROG_TYPE_LWT_XMIT to indicate that skb's dst
+	 *    has changed and appropriate dst_input() or dst_output()
+	 *    action has to be taken (this is an L3 redirect, as
+	 *    opposed to L2 redirect represented by BPF_REDIRECT above).
+	 */
+	BPF_LWT_REROUTE = 128,
 };
 
 struct bpf_sock {
diff --git a/net/core/filter.c b/net/core/filter.c
index 8e587dd1da20..fd3ae092d3d7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4793,7 +4793,13 @@ static int bpf_push_seg6_encap(struct sk_buff *skb, u32 type, void *hdr, u32 len
 }
 #endif /* CONFIG_IPV6_SEG6_BPF */
 
-BPF_CALL_4(bpf_lwt_push_encap, struct sk_buff *, skb, u32, type, void *, hdr,
+static int bpf_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len,
+			     bool ingress)
+{
+	return -EINVAL;  /* Implemented in the next patch. */
+}
+
+BPF_CALL_4(bpf_lwt_in_push_encap, struct sk_buff *, skb, u32, type, void *, hdr,
 	   u32, len)
 {
 	switch (type) {
@@ -4801,14 +4807,41 @@ BPF_CALL_4(bpf_lwt_push_encap, struct sk_buff *, skb, u32, type, void *, hdr,
 	case BPF_LWT_ENCAP_SEG6:
 	case BPF_LWT_ENCAP_SEG6_INLINE:
 		return bpf_push_seg6_encap(skb, type, hdr, len);
+#endif
+#if IS_ENABLED(CONFIG_LWTUNNEL_BPF)
+	case BPF_LWT_ENCAP_IP:
+		return bpf_push_ip_encap(skb, hdr, len, true /* ingress */);
 #endif
 	default:
 		return -EINVAL;
 	}
 }
 
-static const struct bpf_func_proto bpf_lwt_push_encap_proto = {
-	.func		= bpf_lwt_push_encap,
+BPF_CALL_4(bpf_lwt_xmit_push_encap, struct sk_buff *, skb, u32, type,
+	   void *, hdr, u32, len)
+{
+	switch (type) {
+#if IS_ENABLED(CONFIG_LWTUNNEL_BPF)
+	case BPF_LWT_ENCAP_IP:
+		return bpf_push_ip_encap(skb, hdr, len, false /* egress */);
+#endif
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct bpf_func_proto bpf_lwt_in_push_encap_proto = {
+	.func		= bpf_lwt_in_push_encap,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_CONST_SIZE
+};
+
+static const struct bpf_func_proto bpf_lwt_xmit_push_encap_proto = {
+	.func		= bpf_lwt_xmit_push_encap,
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
@@ -5274,7 +5307,8 @@ bool bpf_helper_changes_pkt_data(void *func)
 	    func == bpf_lwt_seg6_adjust_srh ||
 	    func == bpf_lwt_seg6_action ||
 #endif
-	    func == bpf_lwt_push_encap)
+	    func == bpf_lwt_in_push_encap ||
+	    func == bpf_lwt_xmit_push_encap)
 		return true;
 
 	return false;
@@ -5652,7 +5686,7 @@ lwt_in_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
 	switch (func_id) {
 	case BPF_FUNC_lwt_push_encap:
-		return &bpf_lwt_push_encap_proto;
+		return &bpf_lwt_in_push_encap_proto;
 	default:
 		return lwt_out_func_proto(func_id, prog);
 	}
@@ -5688,6 +5722,8 @@ lwt_xmit_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_l4_csum_replace_proto;
 	case BPF_FUNC_set_hash_invalid:
 		return &bpf_set_hash_invalid_proto;
+	case BPF_FUNC_lwt_push_encap:
+		return &bpf_lwt_xmit_push_encap_proto;
 	default:
 		return lwt_out_func_proto(func_id, prog);
 	}
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related

* Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: Andrew Lunn @ 2019-01-24 19:31 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Marek Behun, netdev, David Miller
In-Reply-To: <3c81ed78-4f0e-37b1-710c-6d96c3e76577@gmail.com>

> > 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?
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people, so I
> have not really started doing it just yet :)

Hi Florian

It also has the advantage of being easier to implement. In order to
support 2.5G, i think we need C45 support, etc in the simulator.
Avoiding that would be nice.

	 Andrew

^ permalink raw reply

* [PATCH bpf-next v2 3/3] xsk: add sock_diag interface for AF_XDP
From: bjorn.topel @ 2019-01-24 18:59 UTC (permalink / raw)
  To: ast, daniel, netdev
  Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
In-Reply-To: <20190124185939.23628-1-bjorn.topel@gmail.com>

From: Björn Töpel <bjorn.topel@intel.com>

This patch adds the sock_diag interface for querying sockets from user
space. Tools like iproute2 ss(8) can use this interface to list open
AF_XDP sockets.

The user-space ABI is defined in linux/xdp_diag.h and includes netlink
request and response structs. The request can query sockets and the
response contains socket information about the rings, umems, inode and
more.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/uapi/linux/xdp_diag.h |  72 +++++++++++++
 net/xdp/Kconfig               |   8 ++
 net/xdp/Makefile              |   1 +
 net/xdp/xsk.c                 |   6 +-
 net/xdp/xsk.h                 |  12 +++
 net/xdp/xsk_diag.c            | 191 ++++++++++++++++++++++++++++++++++
 6 files changed, 285 insertions(+), 5 deletions(-)
 create mode 100644 include/uapi/linux/xdp_diag.h
 create mode 100644 net/xdp/xsk.h
 create mode 100644 net/xdp/xsk_diag.c

diff --git a/include/uapi/linux/xdp_diag.h b/include/uapi/linux/xdp_diag.h
new file mode 100644
index 000000000000..78b2591a7782
--- /dev/null
+++ b/include/uapi/linux/xdp_diag.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * xdp_diag: interface for query/monitor XDP sockets
+ * Copyright(c) 2019 Intel Corporation.
+ */
+
+#ifndef _LINUX_XDP_DIAG_H
+#define _LINUX_XDP_DIAG_H
+
+#include <linux/types.h>
+
+struct xdp_diag_req {
+	__u8	sdiag_family;
+	__u8	sdiag_protocol;
+	__u16	pad;
+	__u32	xdiag_ino;
+	__u32	xdiag_show;
+	__u32	xdiag_cookie[2];
+};
+
+struct xdp_diag_msg {
+	__u8	xdiag_family;
+	__u8	xdiag_type;
+	__u16	pad;
+	__u32	xdiag_ino;
+	__u32	xdiag_cookie[2];
+};
+
+#define XDP_SHOW_INFO		(1 << 0) /* Basic information */
+#define XDP_SHOW_RING_CFG	(1 << 1)
+#define XDP_SHOW_UMEM		(1 << 2)
+#define XDP_SHOW_MEMINFO	(1 << 3)
+
+enum {
+	XDP_DIAG_NONE,
+	XDP_DIAG_INFO,
+	XDP_DIAG_UID,
+	XDP_DIAG_RX_RING,
+	XDP_DIAG_TX_RING,
+	XDP_DIAG_UMEM,
+	XDP_DIAG_UMEM_FILL_RING,
+	XDP_DIAG_UMEM_COMPLETION_RING,
+	XDP_DIAG_MEMINFO,
+	__XDP_DIAG_MAX,
+};
+
+#define XDP_DIAG_MAX (__XDP_DIAG_MAX - 1)
+
+struct xdp_diag_info {
+	__u32	ifindex;
+	__u32	queue_id;
+};
+
+struct xdp_diag_ring {
+	__u32	entries; /*num descs */
+};
+
+#define XDP_DU_F_ZEROCOPY (1 << 0)
+
+struct xdp_diag_umem {
+	__u64	size;
+	__u32	id;
+	__u32	num_pages;
+	__u32	chunk_size;
+	__u32	headroom;
+	__u32	ifindex;
+	__u32	queue_id;
+	__u32	flags;
+	__u32	refs;
+};
+
+#endif /* _LINUX_XDP_DIAG_H */
diff --git a/net/xdp/Kconfig b/net/xdp/Kconfig
index 90e4a7152854..0255b33cff4b 100644
--- a/net/xdp/Kconfig
+++ b/net/xdp/Kconfig
@@ -5,3 +5,11 @@ config XDP_SOCKETS
 	help
 	  XDP sockets allows a channel between XDP programs and
 	  userspace applications.
+
+config XDP_SOCKETS_DIAG
+	tristate "XDP sockets: monitoring interface"
+	depends on XDP_SOCKETS
+	default n
+	help
+	  Support for PF_XDP sockets monitoring interface used by the ss tool.
+	  If unsure, say Y.
diff --git a/net/xdp/Makefile b/net/xdp/Makefile
index 04f073146256..59dbfdf93dca 100644
--- a/net/xdp/Makefile
+++ b/net/xdp/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_XDP_SOCKETS) += xsk.o xdp_umem.o xsk_queue.o
+obj-$(CONFIG_XDP_SOCKETS_DIAG) += xsk_diag.o
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 80ca48cefc42..949d3bbccb2f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -27,14 +27,10 @@
 
 #include "xsk_queue.h"
 #include "xdp_umem.h"
+#include "xsk.h"
 
 #define TX_BATCH_SIZE 16
 
-static struct xdp_sock *xdp_sk(struct sock *sk)
-{
-	return (struct xdp_sock *)sk;
-}
-
 bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
 {
 	return READ_ONCE(xs->rx) &&  READ_ONCE(xs->umem) &&
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
new file mode 100644
index 000000000000..ba8120610426
--- /dev/null
+++ b/net/xdp/xsk.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2019 Intel Corporation. */
+
+#ifndef XSK_H_
+#define XSK_H_
+
+static inline struct xdp_sock *xdp_sk(struct sock *sk)
+{
+	return (struct xdp_sock *)sk;
+}
+
+#endif /* XSK_H_ */
diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
new file mode 100644
index 000000000000..661d007c3b28
--- /dev/null
+++ b/net/xdp/xsk_diag.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+/* XDP sockets monitoring support
+ *
+ * Copyright(c) 2019 Intel Corporation.
+ *
+ * Author: Björn Töpel <bjorn.topel@intel.com>
+ */
+
+#include <linux/module.h>
+#include <net/xdp_sock.h>
+#include <linux/xdp_diag.h>
+#include <linux/sock_diag.h>
+
+#include "xsk_queue.h"
+#include "xsk.h"
+
+static int xsk_diag_put_info(const struct xdp_sock *xs, struct sk_buff *nlskb)
+{
+	struct xdp_diag_info di = {};
+
+	di.ifindex = xs->dev ? xs->dev->ifindex : 0;
+	di.queue_id = xs->queue_id;
+	return nla_put(nlskb, XDP_DIAG_INFO, sizeof(di), &di);
+}
+
+static int xsk_diag_put_ring(const struct xsk_queue *queue, int nl_type,
+			     struct sk_buff *nlskb)
+{
+	struct xdp_diag_ring dr = {};
+
+	dr.entries = queue->nentries;
+	return nla_put(nlskb, nl_type, sizeof(dr), &dr);
+}
+
+static int xsk_diag_put_rings_cfg(const struct xdp_sock *xs,
+				  struct sk_buff *nlskb)
+{
+	int err = 0;
+
+	if (xs->rx)
+		err = xsk_diag_put_ring(xs->rx, XDP_DIAG_RX_RING, nlskb);
+	if (!err && xs->tx)
+		err = xsk_diag_put_ring(xs->tx, XDP_DIAG_TX_RING, nlskb);
+	return err;
+}
+
+static int xsk_diag_put_umem(const struct xdp_sock *xs, struct sk_buff *nlskb)
+{
+	struct xdp_umem *umem = xs->umem;
+	struct xdp_diag_umem du = {};
+	int err;
+
+	if (!umem)
+		return 0;
+
+	du.id = umem->id;
+	du.size = umem->size;
+	du.num_pages = umem->npgs;
+	du.chunk_size = (__u32)(~umem->chunk_mask + 1);
+	du.headroom = umem->headroom;
+	du.ifindex = umem->dev ? umem->dev->ifindex : 0;
+	du.queue_id = umem->queue_id;
+	du.flags = 0;
+	if (umem->zc)
+		du.flags |= XDP_DU_F_ZEROCOPY;
+	du.refs = refcount_read(&umem->users);
+
+	err = nla_put(nlskb, XDP_DIAG_UMEM, sizeof(du), &du);
+
+	if (!err && umem->fq)
+		err = xsk_diag_put_ring(xs->tx, XDP_DIAG_UMEM_FILL_RING, nlskb);
+	if (!err && umem->cq) {
+		err = xsk_diag_put_ring(xs->tx, XDP_DIAG_UMEM_COMPLETION_RING,
+					nlskb);
+	}
+	return err;
+}
+
+static int xsk_diag_fill(struct sock *sk, struct sk_buff *nlskb,
+			 struct xdp_diag_req *req,
+			 struct user_namespace *user_ns,
+			 u32 portid, u32 seq, u32 flags, int sk_ino)
+{
+	struct xdp_sock *xs = xdp_sk(sk);
+	struct xdp_diag_msg *msg;
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(nlskb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*msg),
+			flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	msg = nlmsg_data(nlh);
+	memset(msg, 0, sizeof(*msg));
+	msg->xdiag_family = AF_XDP;
+	msg->xdiag_type = sk->sk_type;
+	msg->xdiag_ino = sk_ino;
+	sock_diag_save_cookie(sk, msg->xdiag_cookie);
+
+	if ((req->xdiag_show & XDP_SHOW_INFO) && xsk_diag_put_info(xs, nlskb))
+		goto out_nlmsg_trim;
+
+	if ((req->xdiag_show & XDP_SHOW_INFO) &&
+	    nla_put_u32(nlskb, XDP_DIAG_UID,
+			from_kuid_munged(user_ns, sock_i_uid(sk))))
+		goto out_nlmsg_trim;
+
+	if ((req->xdiag_show & XDP_SHOW_RING_CFG) &&
+	    xsk_diag_put_rings_cfg(xs, nlskb))
+		goto out_nlmsg_trim;
+
+	if ((req->xdiag_show & XDP_SHOW_UMEM) &&
+	    xsk_diag_put_umem(xs, nlskb))
+		goto out_nlmsg_trim;
+
+	if ((req->xdiag_show & XDP_SHOW_MEMINFO) &&
+	    sock_diag_put_meminfo(sk, nlskb, XDP_DIAG_MEMINFO))
+		goto out_nlmsg_trim;
+
+	nlmsg_end(nlskb, nlh);
+	return 0;
+
+out_nlmsg_trim:
+	nlmsg_cancel(nlskb, nlh);
+	return -EMSGSIZE;
+}
+
+static int xsk_diag_dump(struct sk_buff *nlskb, struct netlink_callback *cb)
+{
+	struct xdp_diag_req *req = nlmsg_data(cb->nlh);
+	struct net *net = sock_net(nlskb->sk);
+	int num = 0, s_num = cb->args[0];
+	struct sock *sk;
+
+	mutex_lock(&net->xdp.lock);
+
+	sk_for_each(sk, &net->xdp.list) {
+		if (!net_eq(sock_net(sk), net))
+			continue;
+		if (num++ < s_num)
+			continue;
+
+		if (xsk_diag_fill(sk, nlskb, req,
+				  sk_user_ns(NETLINK_CB(cb->skb).sk),
+				  NETLINK_CB(cb->skb).portid,
+				  cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				  sock_i_ino(sk)) < 0) {
+			num--;
+			break;
+		}
+	}
+
+	mutex_unlock(&net->xdp.lock);
+	cb->args[0] = num;
+	return nlskb->len;
+}
+
+static int xsk_diag_handler_dump(struct sk_buff *nlskb, struct nlmsghdr *hdr)
+{
+	struct netlink_dump_control c = { .dump = xsk_diag_dump };
+	int hdrlen = sizeof(struct xdp_diag_req);
+	struct net *net = sock_net(nlskb->sk);
+
+	if (nlmsg_len(hdr) < hdrlen)
+		return -EINVAL;
+
+	if (!(hdr->nlmsg_flags & NLM_F_DUMP))
+		return -EOPNOTSUPP;
+
+	return netlink_dump_start(net->diag_nlsk, nlskb, hdr, &c);
+}
+
+static const struct sock_diag_handler xsk_diag_handler = {
+	.family = AF_XDP,
+	.dump = xsk_diag_handler_dump,
+};
+
+static int __init xsk_diag_init(void)
+{
+	return sock_diag_register(&xsk_diag_handler);
+}
+
+static void __exit xsk_diag_exit(void)
+{
+	sock_diag_unregister(&xsk_diag_handler);
+}
+
+module_init(xsk_diag_init);
+module_exit(xsk_diag_exit);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, AF_XDP);
-- 
2.19.1


^ permalink raw reply related

* [PATCH bpf-next v2 2/3] xsk: add id to umem
From: bjorn.topel @ 2019-01-24 18:59 UTC (permalink / raw)
  To: ast, daniel, netdev
  Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
In-Reply-To: <20190124185939.23628-1-bjorn.topel@gmail.com>

From: Björn Töpel <bjorn.topel@intel.com>

This commit adds an id to the umem structure. The id uniquely
identifies a umem instance, and will be exposed to user-space via the
socket monitoring interface.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/net/xdp_sock.h |  1 +
 net/xdp/xdp_umem.c     | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 13acb9803a6d..61cf7dbb6782 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -42,6 +42,7 @@ struct xdp_umem {
 	struct work_struct work;
 	struct page **pgs;
 	u32 npgs;
+	int id;
 	struct net_device *dev;
 	struct xdp_umem_fq_reuse *fq_reuse;
 	u16 queue_id;
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index a264cf2accd0..eabdb0f59031 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -13,12 +13,15 @@
 #include <linux/mm.h>
 #include <linux/netdevice.h>
 #include <linux/rtnetlink.h>
+#include <linux/idr.h>
 
 #include "xdp_umem.h"
 #include "xsk_queue.h"
 
 #define XDP_UMEM_MIN_CHUNK_SIZE 2048
 
+static DEFINE_IDA(umem_ida);
+
 void xdp_add_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs)
 {
 	unsigned long flags;
@@ -183,6 +186,8 @@ static void xdp_umem_release(struct xdp_umem *umem)
 
 	xdp_umem_clear_dev(umem);
 
+	ida_simple_remove(&umem_ida, umem->id);
+
 	if (umem->fq) {
 		xskq_destroy(umem->fq);
 		umem->fq = NULL;
@@ -389,8 +394,16 @@ struct xdp_umem *xdp_umem_create(struct xdp_umem_reg *mr)
 	if (!umem)
 		return ERR_PTR(-ENOMEM);
 
+	err = ida_simple_get(&umem_ida, 0, 0, GFP_KERNEL);
+	if (err < 0) {
+		kfree(umem);
+		return ERR_PTR(err);
+	}
+	umem->id = err;
+
 	err = xdp_umem_reg(umem, mr);
 	if (err) {
+		ida_simple_remove(&umem_ida, umem->id);
 		kfree(umem);
 		return ERR_PTR(err);
 	}
-- 
2.19.1


^ permalink raw reply related

* [PATCH bpf-next v2 1/3] net: xsk: track AF_XDP sockets on a per-netns list
From: bjorn.topel @ 2019-01-24 18:59 UTC (permalink / raw)
  To: ast, daniel, netdev
  Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
In-Reply-To: <20190124185939.23628-1-bjorn.topel@gmail.com>

From: Björn Töpel <bjorn.topel@intel.com>

Track each AF_XDP socket in a per-netns list. This will be used later
by the sock_diag interface for querying sockets from userspace.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/net/net_namespace.h |  4 ++++
 include/net/netns/xdp.h     | 13 +++++++++++++
 net/xdp/xsk.c               | 30 ++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 include/net/netns/xdp.h

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 99d4148e0f90..a68ced28d8f4 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -31,6 +31,7 @@
 #include <net/netns/xfrm.h>
 #include <net/netns/mpls.h>
 #include <net/netns/can.h>
+#include <net/netns/xdp.h>
 #include <linux/ns_common.h>
 #include <linux/idr.h>
 #include <linux/skbuff.h>
@@ -160,6 +161,9 @@ struct net {
 #endif
 #if IS_ENABLED(CONFIG_CAN)
 	struct netns_can	can;
+#endif
+#ifdef CONFIG_XDP_SOCKETS
+	struct netns_xdp	xdp;
 #endif
 	struct sock		*diag_nlsk;
 	atomic_t		fnhe_genid;
diff --git a/include/net/netns/xdp.h b/include/net/netns/xdp.h
new file mode 100644
index 000000000000..e5734261ba0a
--- /dev/null
+++ b/include/net/netns/xdp.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NETNS_XDP_H__
+#define __NETNS_XDP_H__
+
+#include <linux/rculist.h>
+#include <linux/mutex.h>
+
+struct netns_xdp {
+	struct mutex		lock;
+	struct hlist_head	list;
+};
+
+#endif /* __NETNS_XDP_H__ */
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index a03268454a27..80ca48cefc42 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -350,6 +350,10 @@ static int xsk_release(struct socket *sock)
 
 	net = sock_net(sk);
 
+	mutex_lock(&net->xdp.lock);
+	sk_del_node_init_rcu(sk);
+	mutex_unlock(&net->xdp.lock);
+
 	local_bh_disable();
 	sock_prot_inuse_add(net, sk->sk_prot, -1);
 	local_bh_enable();
@@ -746,6 +750,10 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
 	mutex_init(&xs->mutex);
 	spin_lock_init(&xs->tx_completion_lock);
 
+	mutex_lock(&net->xdp.lock);
+	sk_add_node_rcu(sk, &net->xdp.list);
+	mutex_unlock(&net->xdp.lock);
+
 	local_bh_disable();
 	sock_prot_inuse_add(net, &xsk_proto, 1);
 	local_bh_enable();
@@ -759,6 +767,23 @@ static const struct net_proto_family xsk_family_ops = {
 	.owner	= THIS_MODULE,
 };
 
+static int __net_init xsk_net_init(struct net *net)
+{
+	mutex_init(&net->xdp.lock);
+	INIT_HLIST_HEAD(&net->xdp.list);
+	return 0;
+}
+
+static void __net_exit xsk_net_exit(struct net *net)
+{
+	WARN_ON_ONCE(!hlist_empty(&net->xdp.list));
+}
+
+static struct pernet_operations xsk_net_ops = {
+	.init = xsk_net_init,
+	.exit = xsk_net_exit,
+};
+
 static int __init xsk_init(void)
 {
 	int err;
@@ -771,8 +796,13 @@ static int __init xsk_init(void)
 	if (err)
 		goto out_proto;
 
+	err = register_pernet_subsys(&xsk_net_ops);
+	if (err)
+		goto out_sk;
 	return 0;
 
+out_sk:
+	sock_unregister(PF_XDP);
 out_proto:
 	proto_unregister(&xsk_proto);
 out:
-- 
2.19.1


^ permalink raw reply related

* [PATCH bpf-next v2 0/3] AF_XDP: add socket monitoring support
From: bjorn.topel @ 2019-01-24 18:59 UTC (permalink / raw)
  To: ast, daniel, netdev
  Cc: Björn Töpel, magnus.karlsson, magnus.karlsson

From: Björn Töpel <bjorn.topel@intel.com>

This series adds an AF_XDP sock_diag interface for querying sockets
from user-space. Tools like iproute2 ss(8) can use this interface to
list open AF_XDP sockets.

The diagnostic provides information about the Rx/Tx/fill/completetion
rings, umem, memory usage and such. For a complete list, please refer
to the xsk_diag.c file.

The AF_XDP sock_diag interface is optional, and can be built as a
module.

A separate patch series, adding ss(8) iproute2 support, will follow.

v1->v2: * Removed extra newline
        * Zero-out all user-space facing structures prior setting the
          members
        * Added explicit "pad" member in _msg struct
        * Removed unused variable "req" in xsk_diag_handler_dump()

Thanks to Daniel for reviewing the series! 

Cheers,
Björn


Björn Töpel (3):
  net: xsk: track AF_XDP sockets on a per-netns list
  xsk: add id to umem
  xsk: add sock_diag interface for AF_XDP

 include/net/net_namespace.h   |   4 +
 include/net/netns/xdp.h       |  13 +++
 include/net/xdp_sock.h        |   1 +
 include/uapi/linux/xdp_diag.h |  72 +++++++++++++
 net/xdp/Kconfig               |   8 ++
 net/xdp/Makefile              |   1 +
 net/xdp/xdp_umem.c            |  13 +++
 net/xdp/xsk.c                 |  36 ++++++-
 net/xdp/xsk.h                 |  12 +++
 net/xdp/xsk_diag.c            | 191 ++++++++++++++++++++++++++++++++++
 10 files changed, 346 insertions(+), 5 deletions(-)
 create mode 100644 include/net/netns/xdp.h
 create mode 100644 include/uapi/linux/xdp_diag.h
 create mode 100644 net/xdp/xsk.h
 create mode 100644 net/xdp/xsk_diag.c

-- 
2.19.1


^ permalink raw reply

* Re: [PATCH bpf-next v2 2/8] libbpf: Add a helper for retrieving a prog via index
From: Jakub Kicinski @ 2019-01-24 18:59 UTC (permalink / raw)
  To: Maciej Fijałkowski
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev
In-Reply-To: <20190124192735.76aaa463@maciek-lenovo>

On Thu, 24 Jan 2019 19:27:35 +0100, Maciej Fijałkowski wrote:
> > > can live with this. In case of typo and program being not found it would be
> > > good to print out the section names to choose from in usage().  
> > 
> > Agree, might be nice as well via libbpf.  
> 
> Would it be acceptable to print them from within libbpf? Something as trivial
> as:
> void
> bpf_object__print_type_sections(struct bpf_object *obj,
> 				enum bpf_prog_type type)
> {
> 	struct bpf_program *prog;
> 
> 	bpf_object__for_each_program(prog, obj) {
> 		if (bpf_program__is_type(prog, type))
> 			pr_info("section %s\n", prog->section_name);
> 	}
> }

Better not.

> Or do we want to add a accessor for section_name and go through programs in
> application?

bpf_program__title(), no?

^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-24 18:56 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: 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.
> 
> > - 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?
> 
> > 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

* Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port
From: Jakub Kicinski @ 2019-01-24 18:50 UTC (permalink / raw)
  To: Vasundhara Volam
  Cc: Michal Kubecek, Netdev, David Miller, michael.chan@broadcom.com,
	Jiri Pirko
In-Reply-To: <CAACQVJq-eop9=K_e6K_aZJdqcnfTjcR0ECpyGCDJf41w0PCxqA@mail.gmail.com>

On Thu, 24 Jan 2019 15:16:27 +0530, Vasundhara Volam wrote:
> > > > The devlink WoL setting does not have to match the ethtool WoL
> > > > setting.  
> > >
> > > IMHO this is not really a problem. We can either use an additional flag
> > > telling kernel/driver if we are setting runtime or persistent WoL mask
> > > or we can pass (up to) two bitmaps.  
> >
> > I think reusing new netlink ethtool with a special flag would be a nice,
> > complete solution.  We could also address link settings this way (which
> > are a pre-requisite for WoL).
> >
> > I have no strong preference on the mechanism, but for ease of setting
> > as well as dumping would it be workable to use a nesting, like this:
> >
> > Run time settings:
> >   [ETHTOOL_SETTINGS_BLA]
> >     [ETHTOOL_BLA_VAL_1]
> >     [ETHTOOL_BLA_VAL_2]
> >     ...
> >
> > Persistent:
> >   [ETHTOOL_PERSISTENT]
> >     [ETHTOOL_SETTINGS_BLA]
> >       [ETHTOOL_BLA_VAL_1]
> >       [ETHTOOL_BLA_VAL_2]
> >
> > IOW encapsulate settings into a "persistent" attribute?  
> Not sure if current devlink framework allows to encapsulate additional
> settings now.
> But we can think of extending it to support this when there is a requirement.

To be clear the question was to Michal and about ethtool netlink, where
this configuration belongs.

^ permalink raw reply

* Re: [PATCH stable 4.4 06/11] ipv6: defrag: drop non-last frags smaller than min mtu
From: Greg KH @ 2019-01-24 18:31 UTC (permalink / raw)
  To: Mao Wenan; +Cc: netdev, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-7-git-send-email-maowenan@huawei.com>

On Wed, Jan 23, 2019 at 10:19:41AM +0800, Mao Wenan wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> [ Upstream commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6 ]
> 
> don't bother with pathological cases, they only waste cycles.
> IPv6 requires a minimum MTU of 1280 so we should never see fragments
> smaller than this (except last frag).
> 
> v3: don't use awkward "-offset + len"
> v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
>     There were concerns that there could be even smaller frags
>     generated by intermediate nodes, e.g. on radio networks.
> 
> Cc: Peter Oskolkov <posk@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
>  net/ipv6/reassembly.c                   | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index 9cd8863..c5033a2 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -602,6 +602,10 @@ struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
>  	hdr = ipv6_hdr(clone);
>  	fhdr = (struct frag_hdr *)skb_transport_header(clone);
>  
> +	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> +	    fhdr->frag_off & htons(IP6_MF))
> +		return -EINVAL;

This backport is incorrect, you should be returning a pointer, right?

How did you test this?  This should have blown up under test :(

I'm going to drop this whole series.  Please fix it up and test it
properly and then resend.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH bpf-next v2 2/8] libbpf: Add a helper for retrieving a prog via index
From: Maciej Fijałkowski @ 2019-01-24 18:27 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Daniel Borkmann; +Cc: ast, netdev, jakub.kicinski
In-Reply-To: <20190124130949.1bfbbf15@redhat.com>

On Thu, 24 Jan 2019 13:09:49 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> On Thu, 24 Jan 2019 12:56:36 +0100
> Daniel Borkmann <daniel@iogearbox.net> wrote:
> 
> > > Please feel free to deprecate or remove the xdp_redirect_cpu --prognum
> > > option.  I would prefer if this was indeed converted to selecting the
> > > program based on the name in the _kern.c file, instead of a number.  
> > 
> > Given this is BPF sample code, there is no need to deprecate, lets just
> > remove --prognum option and add a new one for selection by name.
> 
> I agree, just remove  --prognum option.
> 
Actually we can convert it to --progname and use it for providing the section
name of program to be loaded.

On Wed, 23 Jan 2019 15:11:09 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:

> > can live with this. In case of typo and program being not found it would be
> > good to print out the section names to choose from in usage().
> 
> Agree, might be nice as well via libbpf.

Would it be acceptable to print them from within libbpf? Something as trivial
as:
void
bpf_object__print_type_sections(struct bpf_object *obj,
				enum bpf_prog_type type)
{
	struct bpf_program *prog;

	bpf_object__for_each_program(prog, obj) {
		if (bpf_program__is_type(prog, type))
			pr_info("section %s\n", prog->section_name);
	}
}
Or do we want to add a accessor for section_name and go through programs in
application?

^ permalink raw reply

* KASAN: global-out-of-bounds Read in validate_nla
From: syzbot @ 2019-01-24 18:23 UTC (permalink / raw)
  To: davem, johannes, linux-kernel, linux-wireless, netdev,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    30bac164aca7 Revert "Change mincore() to count "mapped" pa..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17b824c0c00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=505743eba4e4f68
dashboard link: https://syzkaller.appspot.com/bug?extid=4157b036c5f4713b1f2f
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1137adcf400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16431b18c00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4157b036c5f4713b1f2f@syzkaller.appspotmail.com

audit: type=1800 audit(1548293866.971:30): pid=7616 uid=0 auid=4294967295  
ses=4294967295 subj==unconfined op=collect_data cause=failed(directio)  
comm="startpar" name="rmnologin" dev="sda1" ino=2423 res=0
==================================================================
BUG: KASAN: global-out-of-bounds in validate_nla+0x12c4/0x1580  
lib/nlattr.c:169
Read of size 1 at addr ffffffff88f41fc0 by task syz-executor771/7789

CPU: 1 PID: 7789 Comm: syz-executor771 Not tainted 5.0.0-rc3+ #40
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
  print_address_description.cold+0x5/0x20d mm/kasan/report.c:187
  kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
  __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132
  validate_nla+0x12c4/0x1580 lib/nlattr.c:169
  nla_validate+0xc1/0x130 lib/nlattr.c:340
  validate_nla+0x711/0x1580 lib/nlattr.c:247
  __nla_parse+0x206/0x340 lib/nlattr.c:415
  nla_parse+0x45/0x60 lib/nlattr.c:439
  nlmsg_parse include/net/netlink.h:527 [inline]
  nl80211_dump_wiphy_parse.isra.0.constprop.0+0x133/0x610  
net/wireless/nl80211.c:2299
  nl80211_dump_wiphy+0x595/0x760 net/wireless/nl80211.c:2342
  genl_lock_dumpit+0x6d/0xa0 net/netlink/genetlink.c:481
  netlink_dump+0x5f2/0x1070 net/netlink/af_netlink.c:2244
  __netlink_dump_start+0x5b4/0x7e0 net/netlink/af_netlink.c:2352
  genl_family_rcv_msg+0xeb5/0x11a0 net/netlink/genetlink.c:549
  genl_rcv_msg+0xca/0x16c net/netlink/genetlink.c:626
  netlink_rcv_skb+0x17d/0x410 net/netlink/af_netlink.c:2477
  genl_rcv+0x29/0x40 net/netlink/genetlink.c:637
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x574/0x770 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0xa05/0xf90 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x7ec/0x910 net/socket.c:2116
  __sys_sendmsg+0x112/0x270 net/socket.c:2154
  __do_sys_sendmsg net/socket.c:2163 [inline]
  __se_sys_sendmsg net/socket.c:2161 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
  do_syscall_64+0x1a3/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4400d9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffd4f4bf478 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004400d9
RDX: 0000000000000000 RSI: 0000000020000380 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 0000000000000006 R09: 00000000004002c8
R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000401960
R13: 00000000004019f0 R14: 0000000000000000 R15: 0000000000000000

The buggy address belongs to the variable:
  nl80211_pmsr_attr_policy+0x60/0x80

Memory state around the buggy address:
  ffffffff88f41e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffffffff88f41f00: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
> ffffffff88f41f80: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
                                            ^
  ffffffff88f42000: 00 00 00 00 00 00 fa fa fa fa fa fa 00 00 00 00
  ffffffff88f42080: 00 00 fa fa fa fa fa fa 00 00 00 00 fa fa fa fa
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ 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