Netdev List
 help / color / mirror / Atom feed
* i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Benjamin Beckmeyer @ 2019-07-03 13:10 UTC (permalink / raw)
  To: netdev

Hey folks,

I'm having a problem with a custom i.mx6ul board. When DSA is loaded I can't 
get access to the switch via MDIO, but the DSA is working properly. I set up
a bridge for testing and the switch is in forwarding mode and i can ping the 
board. But the MDIO access isn't working at address 2 for the switch. When I 
delete the DSA from the devicetree and start the board up, I can access the 
switch via MDIO.

With DSA up and running:

mii -i 2 0 0x9800
mii -i 2 1
phyid:2, reg:0x01 -> 0x4000
mii -i 2 0 0x9803
mii -i 2 1
phyid:2, reg:0x01 -> 0x4000
mii -i 2 1 0x1883
mii -i 2 1
phyid:2, reg:0x01 -> 0x4000

No DSA:

mii -i 2 0 0x9800
mii -i 2 1
phyid:2, reg:0x01 -> 0xde04
mii -i 2 0 0x9803
mii -i 2 1
phyid:2, reg:0x01 -> 0x3901
mii -i 2 1 0x1883
mii -i 2 1
phyid:2, reg:0x01 -> 0x1883

Here is the device tree for our board:
&mdio0 {
        switch0: switch0@2 {
                compatible = "marvell,mv88e6190";
                reg = <2>;
                pinctrl-0 = <&pinctrl_gpios>;
                reset-gpios = <&gpio4 16 GPIO_ACTIVE_LOW>;

                dsa,member = <0 0>;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@0 {
                                reg = <0>;
                                label = "cpu";
                                ethernet = <&fec1>;
                                phy-mode = "rmii";
                                fixed-link {
                                        speed = <100>;
                                        full-duplex;
                                };
                        };

                        port@1 {
                                reg = <1>;
                                label = "lan1";
                        };
                        port@2 {
                                reg = <2>;
                                label = "lan2";
                        };

                        port@3 {
                                reg = <3>;
                                label = "lan3";
                        };

                        port@4 {
                                reg = <4>;
                                label = "lan4";
                        };

                        port@5 {
                                reg = <5>;
                                label = "lan5";
                        };

                        port@6 {
                                reg = <6>;
                                label = "lan6";
                        };

                        port@7 {
                                reg = <7>;
                                label = "lan7";
                        };
                        port@8 {
                                reg = <8>;
                                label = "lan8";
                        };
                        port@9 {
                                reg = <9>;
                                label = "serdes1";
                                fixed-link {
                                        speed = <1000>;
                                        full-duplex;
                                };
                        };
                        port@10 {
                                reg = <10>;
                                label = "serdes2";
                                fixed-link {
                                        speed = <1000>;
                                        full-duplex;
                                };
                        };
                };
        };
};

On a different custom board we have another switching chip in single chip 
addressing mode the MDIO access works like a charm with activated DSA.

Currently I'm on linux-4.14.118. Other kernels (4.19.55, 5.1.14) I've 
tested stuck at or reboot while DSA is loading. Same devicetree there.
Let me know if you need some more input.

Thanks in advance for your help.

Best regards, 
Benjamin Beckmeyer


^ permalink raw reply

* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-03 13:08 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
	virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <64833f91-02cd-7143-f12e-56ab93b2418d@redhat.com>

On Wed, Jul 03, 2019 at 08:16:23PM +0800, Jason Wang wrote:
> On 2019/7/3 下午7:52, Tiwei Bie wrote:
> > On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
> > > On 2019/7/3 下午5:13, Tiwei Bie wrote:
> > > > Details about this can be found here:
> > > > 
> > > > https://lwn.net/Articles/750770/
> > > > 
> > > > What's new in this version
> > > > ==========================
> > > > 
> > > > A new VFIO device type is introduced - vfio-vhost. This addressed
> > > > some comments from here: https://patchwork.ozlabs.org/cover/984763/
> > > > 
> > > > Below is the updated device interface:
> > > > 
> > > > Currently, there are two regions of this device: 1) CONFIG_REGION
> > > > (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> > > > device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> > > > can be used to notify the device.
> > > > 
> > > > 1. CONFIG_REGION
> > > > 
> > > > The region described by CONFIG_REGION is the main control interface.
> > > > Messages will be written to or read from this region.
> > > > 
> > > > The message type is determined by the `request` field in message
> > > > header. The message size is encoded in the message header too.
> > > > The message format looks like this:
> > > > 
> > > > struct vhost_vfio_op {
> > > > 	__u64 request;
> > > > 	__u32 flags;
> > > > 	/* Flag values: */
> > > >    #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> > > > 	__u32 size;
> > > > 	union {
> > > > 		__u64 u64;
> > > > 		struct vhost_vring_state state;
> > > > 		struct vhost_vring_addr addr;
> > > > 	} payload;
> > > > };
> > > > 
> > > > The existing vhost-kernel ioctl cmds are reused as the message
> > > > requests in above structure.
> > > 
> > > Still a comments like V1. What's the advantage of inventing a new protocol?
> > I'm trying to make it work in VFIO's way..
> > 
> > > I believe either of the following should be better:
> > > 
> > > - using vhost ioctl,  we can start from SET_VRING_KICK/SET_VRING_CALL and
> > > extend it with e.g notify region. The advantages is that all exist userspace
> > > program could be reused without modification (or minimal modification). And
> > > vhost API hides lots of details that is not necessary to be understood by
> > > application (e.g in the case of container).
> > Do you mean reusing vhost's ioctl on VFIO device fd directly,
> > or introducing another mdev driver (i.e. vhost_mdev instead of
> > using the existing vfio_mdev) for mdev device?
> 
> 
> Can we simply add them into ioctl of mdev_parent_ops?

Right, either way, these ioctls have to be and just need to be
added in the ioctl of the mdev_parent_ops. But another thing we
also need to consider is that which file descriptor the userspace
will do the ioctl() on. So I'm wondering do you mean let the
userspace do the ioctl() on the VFIO device fd of the mdev
device?

> 
> 
> > 
[...]
> > > > 3. VFIO interrupt ioctl API
> > > > 
> > > > VFIO interrupt ioctl API is used to setup device interrupts.
> > > > IRQ-bypass can also be supported.
> > > > 
> > > > Currently, the data path interrupt can be configured via the
> > > > VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
> > > 
> > > How about DMA API? Do you expect to use VFIO IOMMU API or using vhost
> > > SET_MEM_TABLE? VFIO IOMMU API is more generic for sure but with
> > > SET_MEM_TABLE DMA can be done at the level of parent device which means it
> > > can work for e.g the card with on-chip IOMMU.
> > Agree. In this RFC, it assumes userspace will use VFIO IOMMU API
> > to do the DMA programming. But like what you said, there could be
> > a problem when using cards with on-chip IOMMU.
> 
> 
> Yes, another issue is SET_MEM_TABLE can not be used to update just a part of
> the table. This seems less flexible than VFIO API but it could be extended.

Agree.

> 
> 
> > 
> > > And what's the plan for vIOMMU?
> > As this RFC assumes userspace will use VFIO IOMMU API, userspace
> > just needs to follow the same way like what vfio-pci device does
> > in QEMU to support vIOMMU.
> 
> 
> Right, this is more a question for the qemu part. It means it needs to go
> for ordinary VFIO path to get all notifiers/listeners support from vIOMMU.

Yeah.

> 
> 
> > 
> > > 
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > >    drivers/vhost/Makefile     |   2 +
> > > >    drivers/vhost/vdpa.c       | 770 +++++++++++++++++++++++++++++++++++++
> > > >    include/linux/vdpa_mdev.h  |  72 ++++
> > > >    include/uapi/linux/vfio.h  |  19 +
> > > >    include/uapi/linux/vhost.h |  25 ++
> > > >    5 files changed, 888 insertions(+)
> > > >    create mode 100644 drivers/vhost/vdpa.c
> > > >    create mode 100644 include/linux/vdpa_mdev.h
> > > 
> > > We probably need some sample parent device implementation. It could be a
> > > software datapath like e.g we can start from virtio-net device in guest or a
> > > vhost/tap on host.
> > Yeah, something like this would be interesting!
> 
> 
> Plan to do something like that :) ?

I don't have a plan yet.. But it's something that can be done I think.

Thanks,
Tiwei

> 
> Thanks
> 
> 
> > 
> > Thanks,
> > Tiwei
> > 
> > > Thanks
> > > 
> > > 

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Jes Sorensen @ 2019-07-03 13:01 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team
In-Reply-To: <CAB4CAwcEdcg91Bgb+JoCdk_zQKsWT-K+cb07-5mrrx+__X2RMA@mail.gmail.com>

On 7/2/19 11:25 PM, Chris Chiu wrote:
> On Tue, Jul 2, 2019 at 8:44 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>>
>> On 6/27/19 5:52 AM, Chris Chiu wrote:
>>> The WiFi tx power of RTL8723BU is extremely low after booting. So
>>> the WiFi scan gives very limited AP list and it always fails to
>>> connect to the selected AP. This module only supports 1x1 antenna
>>> and the antenna is switched to bluetooth due to some incorrect
>>> register settings.
>>>
>>> This commit hand over the antenna control to PTA, the wifi signal
>>> will be back to normal and the bluetooth scan can also work at the
>>> same time. However, the btcoexist still needs to be handled under
>>> different circumstances. If there's a BT connection established,
>>> the wifi still fails to connect until disconneting the BT.
>>>
>>> Signed-off-by: Chris Chiu <chiu@endlessm.com>
>>> ---
>>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 9 ++++++---
>>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 3 ++-
>>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> index 3adb1d3d47ac..6c3c70d93ac1 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>>>       /*
>>>        * WLAN action by PTA
>>>        */
>>> -     rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
>>> +     rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
>>>
>>>       /*
>>>        * BT select S0/S1 controlled by WiFi
>>> @@ -1568,9 +1568,12 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>>>       rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
>>>
>>>       /*
>>> -      * 0x280, 0x00, 0x200, 0x80 - not clear
>>> +      * Different settings per different antenna position.
>>> +      * Antenna switch to BT: 0x280, 0x00 (inverse)
>>> +      * Antenna switch to WiFi: 0x0, 0x280 (inverse)
>>> +      * Antenna controlled by PTA: 0x200, 0x80 (inverse)
>>>        */
>>> -     rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
>>> +     rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
>>>
>>>       /*
>>>        * Software control, antenna at WiFi side
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index 8136e268b4e6..87b2179a769e 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -3891,12 +3891,13 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>>>
>>>       /* Check if MAC is already powered on */
>>>       val8 = rtl8xxxu_read8(priv, REG_CR);
>>> +     val16 = rtl8xxxu_read16(priv, REG_SYS_CLKR);
>>>
>>>       /*
>>>        * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>>>        * initialized. First MAC returns 0xea, second MAC returns 0x00
>>>        */
>>> -     if (val8 == 0xea)
>>> +     if (val8 == 0xea || !(val16 & BIT(11)))
>>>               macpower = false;
>>>       else
>>>               macpower = true;
>>
>> This part I would like to ask you take a good look at the other chips to
>> make sure you don't break support for 8192cu, 8723au, 8188eu with this.
>>
>> Cheers,
>> Jes
> 
> I checked the vendor code of 8192cu and 8188eu, they don't have this part
> of code to check the REG_CR before power on sequence. I can only find
> similar code in rtl8723be.
> if (tmp_u1b != 0 && tmp_u1b !=0xea)
>     rtlhal->mac_func_enable = true;
> 
> By definition, the BIT(11) of REG_SYS_CLKR in rtl8xxxu_regs.h is
> SYS_CLK_MAC_CLK_ENABLE. It seems to make sense to check this value
> for macpower no matter what chip it is. I think I can make it more
> self-expressive
> as down below.
> 
>  if (val8 == 0xea || !(val16 & SYS_CLK_MAC_CLK_ENABLE))

Yes, please always use the descriptive defines rather than hard coding
the bit numbers.

> And per the comment, this code is for 92DU-VC S3 hang problem and I think an
> OR check for SYS_CLK_MAC_CLK_ENABLE is still safe for this.

Sounds reasonable - keep in mind that some of these bugs may have been
fixed for one chip, and then just copied forward.

Cheers,
Jes

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Jes Sorensen @ 2019-07-03 12:59 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Chris Chiu, Kalle Valo, David Miller, linux-wireless, netdev,
	Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <CAD8Lp47mWH1-VsZaHr6_qmSU2EEOr9tQJ3CUhfi_JkQGgKpegA@mail.gmail.com>

On 7/3/19 3:42 AM, Daniel Drake wrote:
> On Tue, Jul 2, 2019 at 8:42 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> We definitely don't want to bring over the vendor code, since it's a
>> pile of spaghetti, but we probably need to get something sorted. This
>> went down the drain when the bluetooth driver was added without taking
>> it into account - long after this driver was merged.
> 
> Yeah, I didn't mean bring over quite so literally.. Chris is studying
> it and figuring out the neatest way to reimplement the required bits.
> 
> As for the relationship with bluetooth.. actually the bug that Chris
> is working on here is that the rtl8xxxu wifi signal is totally
> unusable *until* the bluetooth driver is loaded.

So this is not my experience at all from when I wrote the code. The
8723bu dongle I used for it came up just fine.

> Once the bluetooth driver is loaded, at the point of bluetooth
> firmware upload, the rtl8xxxu signal magiaclly strength becomes good.
> I think this is consistent with other rtl8xxxu problem reports that we
> saw lying around, although they had not been diagnosed in so much
> detail.

See this is the very opposite of what I have experienced. The bluetooth
driver ruins the signal when it's loaded with my dongle.

> The rtl8723bu vendor driver does not suffer this problem, it works
> fine with or without the bluetooth driver in place.

My point is this seems to be very dongle dependent :( We have to be
careful not breaking it for some users while fixing it for others.

Cheers,
Jes

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on s390
From: Daniel Borkmann @ 2019-07-03 12:56 UTC (permalink / raw)
  To: Ilya Leoshkevich, bpf, netdev
In-Reply-To: <20190702153908.41562-1-iii@linux.ibm.com>

On 07/02/2019 05:39 PM, Ilya Leoshkevich wrote:
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
> 
> Pass -D__TARGET_ARCH_$(ARCH) to selftests in order to choose a proper
> PT_REGS_RC variant.
> 
> Fix s930 -> s390 typo.
> 
> On s390, provide the forward declaration of struct pt_regs and cast it
> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
> of the full struct pt_regs, s390 exposes only its first field
> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
> (in selftests) and kernel (in samples) headers.
> 
> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390, x86
> provides struct pt_regs to both userspace and kernel, however, with
> different field names.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

This doesn't apply cleanly to bpf-next, please rebase. I also think this
should be ideally split into multiple patches, seems like 4 different
issues which you are addressing in this single patch.

Thanks,
Daniel

^ permalink raw reply

* RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-03 12:56 UTC (permalink / raw)
  To: Jiri Pirko, Jakub Kicinski
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Michal Kalderon,
	Ariel Elior
In-Reply-To: <20190620133748.GD2504@nanopsycho>

> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, June 20, 2019 7:08 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
> 
> Thu, Jun 20, 2019 at 02:09:29PM CEST, skalluru@marvell.com wrote:
> >> -----Original Message-----
> >> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> Sent: Tuesday, June 18, 2019 4:24 AM
> >> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> >> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> >> <jiri@resnulli.us>
> >> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> >> configuration attributes.
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> >> > This patch adds implementation for devlink callbacks for reading/
> >> > configuring the device attributes.
> >> >
> >> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> >>
> >> You need to provide documentation for your parameters, plus some of
> >> them look like they should potentially be port params, not device params.
> >
> >Thanks a lot for your review. Will add the required documentation. In case
> of Marvell adapter, any of the device/adapter/port parameters can be
> read/configurable via any PF (ethdev) on the port. Hence adding the
> commands at device level. Hope this is fine.
> 
> No it is not. Port param should be port param.
> 
> Also please be careful not to add any generic param as driver specific.
> 
> Thanks!
Apologies for bringing this topic again. From the driver(s) code paths/'devlink man pages', I understood that devlink-port object is an entity on top of the PCI bus device.
Some drivers say NFP represents vnics (on pci-dev) as a devlink-ports and, some represents (virtual?) ports on the PF/device as devlink-ports.
In the case of Marvell NIC driver, we don't have [port] partitioning of the PCI device. And the config attributes are specific to PCI-device (not the vports/vnics of PF).
Hence I didn't see a need for creating devlink-port objects in the system for Marvell NICs. And planning to add the config attributes to 'devlink-dev' object.
Please let me know if my understanding and the proposal is ok?

Code paths which use devlink-port:
   mlx4_load_one(struct pci_dev *pdev):  mlx4_init_port_info(1 .. dev->caps.num_ports) -> devlink_port_register()
   nfp_net_pf_init_vnics (1 .. pf->vnics) -> nfp_net_pf_init_vnic() -> nfp_devlink_port_register()
   nsim_dev_probe (1 .. nsim_bus_dev->port_count) -> __nsim_dev_port_add() -> devlink_port_register ()

man page for 'devlink-port':
   devlink port set - change devlink port attributes
       DEV/PORT_INDEX - specifies the devlink port to operate on.

       devlink port show pci/0000:01:00.0/1
           Shows the state of specified devlink port.

^ permalink raw reply

* [PATCH bpf-next v3] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-07-03 12:52 UTC (permalink / raw)
  To: netdev
  Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
	magnus.karlsson

When an AF_XDP application received X packets, it does not mean X
frames can be stuffed into the producer ring. To make it easier for
AF_XDP applications this API allows them to check how many frames can
be added into the ring.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---

v2 -> v3
 - Removed cache by pass option

v1 -> v2
 - Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
 - Add caching so it will only touch global state when needed

 tools/lib/bpf/xsk.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..3411556e04d9 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -76,7 +76,7 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
 	return &descs[idx & rx->mask];
 }
 
-static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
+static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
 {
 	__u32 free_entries = r->cached_cons - r->cached_prod;
 
@@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
 static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
 					    size_t nb, __u32 *idx)
 {
-	if (xsk_prod_nb_free(prod, nb) < nb)
+	if (xsk_prod__nb_free(prod, nb) < nb)
 		return 0;
 
 	*idx = prod->cached_prod;
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Linus Walleij @ 2019-07-03 12:41 UTC (permalink / raw)
  To: Pawel Dembicki
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
	Rob Herring, Mark Rutland, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190703085757.1027-1-paweldembicki@gmail.com>

On Wed, Jul 3, 2019 at 10:58 AM Pawel Dembicki <paweldembicki@gmail.com> wrote:

> This commit introduce how to use vsc73xx platform driver.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
> Changes in v2:
> - Drop -spi and -platform suffix
> - Change commit message

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v5 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Daniel Borkmann @ 2019-07-03 12:39 UTC (permalink / raw)
  To: Andrii Nakryiko, andrii.nakryiko, bpf, netdev, ast, kernel-team,
	yhs
In-Reply-To: <20190701235903.660141-5-andriin@fb.com>

On 07/02/2019 01:58 AM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> ---
>  tools/lib/bpf/libbpf.c   | 169 +++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |   7 ++
>  tools/lib/bpf/libbpf.map |   2 +
>  3 files changed, 178 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index bcaa294f819d..7b6142408b15 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4021,6 +4021,175 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
>  	return (struct bpf_link *)link;
>  }
>  
> +/*
> + * this function is expected to parse integer in the range of [0, 2^31-1] from
> + * given file using scanf format string fmt. If actual parsed value is
> + * negative, the result might be indistinguishable from error
> + */
> +static int parse_uint_from_file(const char *file, const char *fmt)
> +{
> +	char buf[STRERR_BUFSIZE];
> +	int err, ret;
> +	FILE *f;
> +
> +	f = fopen(file, "r");
> +	if (!f) {
> +		err = -errno;
> +		pr_debug("failed to open '%s': %s\n", file,
> +			 libbpf_strerror_r(err, buf, sizeof(buf)));
> +		return err;
> +	}
> +	err = fscanf(f, fmt, &ret);
> +	if (err != 1) {
> +		err = err == EOF ? -EIO : -errno;
> +		pr_debug("failed to parse '%s': %s\n", file,
> +			libbpf_strerror_r(err, buf, sizeof(buf)));
> +		fclose(f);
> +		return err;
> +	}
> +	fclose(f);
> +	return ret;
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +
> +	return parse_uint_from_file(file, "%d\n");
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +
> +	return parse_uint_from_file(file, "%d\n");
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +
> +	return parse_uint_from_file(file, "config:%d\n");
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +
> +	return parse_uint_from_file(file, "config:%d\n");
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> +				 uint64_t offset, int pid)
> +{
> +	struct perf_event_attr attr = {};
> +	char errmsg[STRERR_BUFSIZE];
> +	int type, pfd, err;
> +
> +	type = uprobe ? determine_uprobe_perf_type()
> +		      : determine_kprobe_perf_type();
> +	if (type < 0) {
> +		pr_warning("failed to determine %s perf type: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> +		return type;
> +	}
> +	if (retprobe) {
> +		int bit = uprobe ? determine_uprobe_retprobe_bit()
> +				 : determine_kprobe_retprobe_bit();
> +
> +		if (bit < 0) {
> +			pr_warning("failed to determine %s retprobe bit: %s\n",
> +				   uprobe ? "uprobe" : "kprobe",
> +				   libbpf_strerror_r(bit, errmsg,
> +						     sizeof(errmsg)));
> +			return bit;
> +		}
> +		attr.config |= 1 << bit;
> +	}
> +	attr.size = sizeof(attr);
> +	attr.type = type;
> +	attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> +	attr.config2 = offset;		       /* kprobe_addr or probe_offset */
> +
> +	/* pid filter is meaningful only for uprobes */
> +	pfd = syscall(__NR_perf_event_open, &attr,
> +		      pid < 0 ? -1 : pid /* pid */,
> +		      pid == -1 ? 0 : -1 /* cpu */,
> +		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> +	if (pfd < 0) {
> +		err = -errno;
> +		pr_warning("%s perf_event_open() failed: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> +					    bool retprobe,
> +					    const char *func_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link *link;
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> +				    0 /* offset */, -1 /* pid */);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link = bpf_program__attach_perf_event(prog, pfd);
> +	if (IS_ERR(link)) {
> +		close(pfd);
> +		err = PTR_ERR(link);
> +		pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return link;
> +	}
> +	return link;
> +}
> +
> +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> +					    bool retprobe, pid_t pid,
> +					    const char *binary_path,
> +					    size_t func_offset)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link *link;
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> +				    binary_path, func_offset, pid);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link = bpf_program__attach_perf_event(prog, pfd);
> +	if (IS_ERR(link)) {
> +		close(pfd);
> +		err = PTR_ERR(link);
> +		pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return link;
> +	}
> +	return link;
> +}

Hm, this only addresses half the feedback I had in prior version [0]. Patch 2/9
with bpf_link with destructor looks good to me, but my feedback from back then was
that all the kprobe/uprobe/tracepoint/raw_tracepoint should be split API-wise, so
you'll end up with something like the below, that is, 1) a set of functions that
only /create/ the bpf_link handle /once/, and 2) a helper that allows /attaching/
progs to one or multiple bpf_links. The set of APIs would look like:

struct bpf_link *bpf_link__create_kprobe(bool retprobe, const char *func_name);
struct bpf_link *bpf_link__create_uprobe(bool retprobe, pid_t pid,
					 const char *binary_path,
					 size_t func_offset);
int bpf_program__attach_to_link(struct bpf_link *link, struct bpf_program *prog);
int bpf_link__destroy(struct bpf_link *link);

This seems much more natural to me. Right now you sort of do both in one single API.
Detangling the bpf_program__attach_{uprobe,kprobe}() would also avoid that you have
to redo all the perf_event_open_probe() work over and over in order to get the pfd
context where you can later attach something to. Given bpf_program__attach_to_link()
API, you also wouldn't need to expose the bpf_program__attach_perf_event() from
patch 3/9. Thoughts?

  [0] https://lore.kernel.org/bpf/a7780057-1d70-9ace-960b-ff65867dc277@iogearbox.net/

>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>  			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1bf66c4a9330..bd767cc11967 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>  
>  LIBBPF_API struct bpf_link *
>  bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> +			   const char *func_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> +			   pid_t pid, const char *binary_path,
> +			   size_t func_offset);
>  
>  struct bpf_insn;
>  
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 756f5aa802e9..57a40fb60718 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
>  	global:
>  		bpf_link__destroy;
>  		bpf_object__load_xattr;
> +		bpf_program__attach_kprobe;
>  		bpf_program__attach_perf_event;
> +		bpf_program__attach_uprobe;
>  		btf_dump__dump_type;
>  		btf_dump__free;
>  		btf_dump__new;
> 


^ permalink raw reply

* Re: [PATCH net] sctp: count data bundling sack chunk for outctrlchunks
From: Marcelo Ricardo Leitner @ 2019-07-03 12:26 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <62e917e312bc582e96fa19b502561e37ca7f91a6.1562149220.git.lucien.xin@gmail.com>

On Wed, Jul 03, 2019 at 06:20:20PM +0800, Xin Long wrote:
> Now all ctrl chunks are counted for asoc stats.octrlchunks and net
> SCTP_MIB_OUTCTRLCHUNKS either after queuing up or bundling, other
> than the chunk maked and bundled in sctp_packet_bundle_sack, which
> caused 'outctrlchunks' not consistent with 'inctrlchunks' in peer.
> 
> This issue exists since very beginning, here to fix it by increasing
> both net SCTP_MIB_OUTCTRLCHUNKS and asoc stats.octrlchunks when sack
> chunk is maked and bundled in sctp_packet_bundle_sack.
> 
> Reported-by: Ja Ram Jeon <jajeon@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/output.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index e0c2747..dbda7e7 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -282,6 +282,9 @@ static enum sctp_xmit sctp_packet_bundle_sack(struct sctp_packet *pkt,
>  					sctp_chunk_free(sack);
>  					goto out;
>  				}
> +				SCTP_INC_STATS(sock_net(asoc->base.sk),
> +					       SCTP_MIB_OUTCTRLCHUNKS);
> +				asoc->stats.octrlchunks++;
>  				asoc->peer.sack_needed = 0;
>  				if (del_timer(timer))
>  					sctp_association_put(asoc);
> -- 
> 2.1.0
> 

^ permalink raw reply

* Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-07-03 12:23 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: wenxu, fw, netfilter-devel, netdev
In-Reply-To: <366e228f-7253-e388-4799-f0f9c18d1111@cumulusnetworks.com>

On Wed, Jul 03, 2019 at 03:08:01PM +0300, Nikolay Aleksandrov wrote:
> On 28/06/2019 03:49, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> > 
> > This patch provide a meta to get the bridge vlan proto
> > 
> > nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
> > 
> > Signed-off-by: wenxu <wenxu@ucloud.cn>
> > ---
> >  include/uapi/linux/netfilter/nf_tables.h | 2 ++
> >  net/netfilter/nft_meta.c                 | 9 +++++++++
> >  2 files changed, 11 insertions(+)
> > 
> 
> Hi,
> When using the internal bridge API outside of the bridge I'd advise you to CC bridge
> maintainers as well.

Will keep this mind, thanks.

> This patch is clearly wrong since you cannot access the vlan
> fields directly because bridge vlan support might be disabled from the kernel config
> as Pablo has noticed as well. In general I'd try to avoid using the internal API directly,
> but that is a different matter.

BROPT_VLAN_ENABLED is exposed through netlink and sysfs, and this only
consults the value. I guess you refer to the fact that...

> Please consult with include/linux/if_bridge.h for exported
> functions that are supposed to be visible outside of the bridge, if you need anything else
> make sure to add support for it there. The usage of br_opt_get directly for example must
> be changed to br_vlan_enabled().

Indeed... this patch should be using br_vlan_enabled() instead.

Thanks.

^ permalink raw reply

* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Jason Wang @ 2019-07-03 12:16 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
	virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <20190703115245.GA22374@___>


On 2019/7/3 下午7:52, Tiwei Bie wrote:
> On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
>> On 2019/7/3 下午5:13, Tiwei Bie wrote:
>>> Details about this can be found here:
>>>
>>> https://lwn.net/Articles/750770/
>>>
>>> What's new in this version
>>> ==========================
>>>
>>> A new VFIO device type is introduced - vfio-vhost. This addressed
>>> some comments from here: https://patchwork.ozlabs.org/cover/984763/
>>>
>>> Below is the updated device interface:
>>>
>>> Currently, there are two regions of this device: 1) CONFIG_REGION
>>> (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
>>> device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
>>> can be used to notify the device.
>>>
>>> 1. CONFIG_REGION
>>>
>>> The region described by CONFIG_REGION is the main control interface.
>>> Messages will be written to or read from this region.
>>>
>>> The message type is determined by the `request` field in message
>>> header. The message size is encoded in the message header too.
>>> The message format looks like this:
>>>
>>> struct vhost_vfio_op {
>>> 	__u64 request;
>>> 	__u32 flags;
>>> 	/* Flag values: */
>>>    #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
>>> 	__u32 size;
>>> 	union {
>>> 		__u64 u64;
>>> 		struct vhost_vring_state state;
>>> 		struct vhost_vring_addr addr;
>>> 	} payload;
>>> };
>>>
>>> The existing vhost-kernel ioctl cmds are reused as the message
>>> requests in above structure.
>>
>> Still a comments like V1. What's the advantage of inventing a new protocol?
> I'm trying to make it work in VFIO's way..
>
>> I believe either of the following should be better:
>>
>> - using vhost ioctl,  we can start from SET_VRING_KICK/SET_VRING_CALL and
>> extend it with e.g notify region. The advantages is that all exist userspace
>> program could be reused without modification (or minimal modification). And
>> vhost API hides lots of details that is not necessary to be understood by
>> application (e.g in the case of container).
> Do you mean reusing vhost's ioctl on VFIO device fd directly,
> or introducing another mdev driver (i.e. vhost_mdev instead of
> using the existing vfio_mdev) for mdev device?


Can we simply add them into ioctl of mdev_parent_ops?


>
>> - using PCI layout, then you don't even need to re-invent notifiy region at
>> all and we can pass-through them to guest.
> Like what you said previously, virtio has transports other than PCI.
> And it will look a bit odd when using transports other than PCI..


Yes.


>
>> Personally, I prefer vhost ioctl.
> +1
>
>>
> [...]
>>> 3. VFIO interrupt ioctl API
>>>
>>> VFIO interrupt ioctl API is used to setup device interrupts.
>>> IRQ-bypass can also be supported.
>>>
>>> Currently, the data path interrupt can be configured via the
>>> VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
>>
>> How about DMA API? Do you expect to use VFIO IOMMU API or using vhost
>> SET_MEM_TABLE? VFIO IOMMU API is more generic for sure but with
>> SET_MEM_TABLE DMA can be done at the level of parent device which means it
>> can work for e.g the card with on-chip IOMMU.
> Agree. In this RFC, it assumes userspace will use VFIO IOMMU API
> to do the DMA programming. But like what you said, there could be
> a problem when using cards with on-chip IOMMU.


Yes, another issue is SET_MEM_TABLE can not be used to update just a 
part of the table. This seems less flexible than VFIO API but it could 
be extended.


>
>> And what's the plan for vIOMMU?
> As this RFC assumes userspace will use VFIO IOMMU API, userspace
> just needs to follow the same way like what vfio-pci device does
> in QEMU to support vIOMMU.


Right, this is more a question for the qemu part. It means it needs to 
go for ordinary VFIO path to get all notifiers/listeners support from 
vIOMMU.


>
>>
>>> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>> ---
>>>    drivers/vhost/Makefile     |   2 +
>>>    drivers/vhost/vdpa.c       | 770 +++++++++++++++++++++++++++++++++++++
>>>    include/linux/vdpa_mdev.h  |  72 ++++
>>>    include/uapi/linux/vfio.h  |  19 +
>>>    include/uapi/linux/vhost.h |  25 ++
>>>    5 files changed, 888 insertions(+)
>>>    create mode 100644 drivers/vhost/vdpa.c
>>>    create mode 100644 include/linux/vdpa_mdev.h
>>
>> We probably need some sample parent device implementation. It could be a
>> software datapath like e.g we can start from virtio-net device in guest or a
>> vhost/tap on host.
> Yeah, something like this would be interesting!


Plan to do something like that :) ?

Thanks


>
> Thanks,
> Tiwei
>
>> Thanks
>>
>>

^ permalink raw reply

* [PATCH bpf v2] xdp: fix race on generic receive path
From: Ilya Maximets @ 2019-07-03 12:09 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, bpf, xdp-newbies, David S. Miller,
	Björn Töpel, Magnus Karlsson, Jonathan Lemon,
	Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
	Ilya Maximets
In-Reply-To: <CGME20190703120922eucas1p2d97e3b994425ecdd2dadd13744ac2a77@eucas1p2.samsung.com>

Unlike driver mode, generic xdp receive could be triggered
by different threads on different CPU cores at the same time
leading to the fill and rx queue breakage. For example, this
could happen while sending packets from two processes to the
first interface of veth pair while the second part of it is
open with AF_XDP socket.

Need to take a lock for each generic receive to avoid race.

Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

Version 2:
    * spin_lock_irqsave --> spin_lock_bh.

 include/net/xdp_sock.h |  2 ++
 net/xdp/xsk.c          | 31 ++++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index d074b6d60f8a..ac3c047d058c 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -67,6 +67,8 @@ struct xdp_sock {
 	 * in the SKB destructor callback.
 	 */
 	spinlock_t tx_completion_lock;
+	/* Protects generic receive. */
+	spinlock_t rx_lock;
 	u64 rx_dropped;
 };
 
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index a14e8864e4fa..5e0637db92ea 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -123,13 +123,17 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
 	u64 addr;
 	int err;
 
-	if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
-		return -EINVAL;
+	spin_lock_bh(&xs->rx_lock);
+
+	if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index) {
+		err = -EINVAL;
+		goto out_unlock;
+	}
 
 	if (!xskq_peek_addr(xs->umem->fq, &addr) ||
 	    len > xs->umem->chunk_size_nohr - XDP_PACKET_HEADROOM) {
-		xs->rx_dropped++;
-		return -ENOSPC;
+		err = -ENOSPC;
+		goto out_drop;
 	}
 
 	addr += xs->umem->headroom;
@@ -138,13 +142,21 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
 	memcpy(buffer, xdp->data_meta, len + metalen);
 	addr += metalen;
 	err = xskq_produce_batch_desc(xs->rx, addr, len);
-	if (!err) {
-		xskq_discard_addr(xs->umem->fq);
-		xsk_flush(xs);
-		return 0;
-	}
+	if (err)
+		goto out_drop;
+
+	xskq_discard_addr(xs->umem->fq);
+	xskq_produce_flush_desc(xs->rx);
 
+	spin_unlock_bh(&xs->rx_lock);
+
+	xs->sk.sk_data_ready(&xs->sk);
+	return 0;
+
+out_drop:
 	xs->rx_dropped++;
+out_unlock:
+	spin_unlock_bh(&xs->rx_lock);
 	return err;
 }
 
@@ -765,6 +777,7 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
 
 	xs = xdp_sk(sk);
 	mutex_init(&xs->mutex);
+	spin_lock_init(&xs->rx_lock);
 	spin_lock_init(&xs->tx_completion_lock);
 
 	mutex_lock(&net->xdp.lock);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: Nikolay Aleksandrov @ 2019-07-03 12:08 UTC (permalink / raw)
  To: wenxu, pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561682975-21790-1-git-send-email-wenxu@ucloud.cn>

On 28/06/2019 03:49, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> This patch provide a meta to get the bridge vlan proto
> 
> nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  include/uapi/linux/netfilter/nf_tables.h | 2 ++
>  net/netfilter/nft_meta.c                 | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 

Hi,
When using the internal bridge API outside of the bridge I'd advise you to CC bridge
maintainers as well. This patch is clearly wrong since you cannot access the vlan
fields directly because bridge vlan support might be disabled from the kernel config
as Pablo has noticed as well. In general I'd try to avoid using the internal API directly,
but that is a different matter. Please consult with include/linux/if_bridge.h for exported
functions that are supposed to be visible outside of the bridge, if you need anything else
make sure to add support for it there. The usage of br_opt_get directly for example must
be changed to br_vlan_enabled(). 

Thanks,
 Nik



^ permalink raw reply

* Re: [PATCH bpf] xdp: fix race on generic receive path
From: Ilya Maximets @ 2019-07-03 12:03 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
	Björn Töpel, Magnus Karlsson, Jonathan Lemon,
	Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190702174014.005a3166@cakuba.netronome.com>

On 03.07.2019 3:40, Jakub Kicinski wrote:
> On Tue,  2 Jul 2019 17:36:34 +0300, Ilya Maximets wrote:
>> Unlike driver mode, generic xdp receive could be triggered
>> by different threads on different CPU cores at the same time
>> leading to the fill and rx queue breakage. For example, this
>> could happen while sending packets from two processes to the
>> first interface of veth pair while the second part of it is
>> open with AF_XDP socket.
>>
>> Need to take a lock for each generic receive to avoid race.
>>
>> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> 
>> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
>> index a14e8864e4fa..19f41d2b670c 100644
>> --- a/net/xdp/xsk.c
>> +++ b/net/xdp/xsk.c
>> @@ -119,17 +119,22 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
>>  {
>>  	u32 metalen = xdp->data - xdp->data_meta;
>>  	u32 len = xdp->data_end - xdp->data;
>> +	unsigned long flags;
>>  	void *buffer;
>>  	u64 addr;
>>  	int err;
>>  
>> -	if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
>> -		return -EINVAL;
>> +	spin_lock_irqsave(&xs->rx_lock, flags);
> 
> Why _irqsave, rather than _bh?

Yes, spin_lock_bh() is enough here. Will change in v2.
Thanks.

> 
>> +	if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index) {
>> +		err = -EINVAL;
>> +		goto out_unlock;
>> +	}
>>  
>>  	if (!xskq_peek_addr(xs->umem->fq, &addr) ||
>>  	    len > xs->umem->chunk_size_nohr - XDP_PACKET_HEADROOM) {
>> -		xs->rx_dropped++;
>> -		return -ENOSPC;
>> +		err = -ENOSPC;
>> +		goto out_drop;
>>  	}
>>  
>>  	addr += xs->umem->headroom;
> 
> 
> 

^ permalink raw reply

* Re: [RFC v2] vhost: introduce mdev based hardware vhost backend
From: Tiwei Bie @ 2019-07-03 11:52 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, alex.williamson, maxime.coquelin, linux-kernel, kvm,
	virtualization, netdev, dan.daly, cunming.liang, zhihong.wang
In-Reply-To: <7b8279b2-aa7e-7adc-eeff-20dfaf4400d0@redhat.com>

On Wed, Jul 03, 2019 at 06:09:51PM +0800, Jason Wang wrote:
> On 2019/7/3 下午5:13, Tiwei Bie wrote:
> > Details about this can be found here:
> > 
> > https://lwn.net/Articles/750770/
> > 
> > What's new in this version
> > ==========================
> > 
> > A new VFIO device type is introduced - vfio-vhost. This addressed
> > some comments from here: https://patchwork.ozlabs.org/cover/984763/
> > 
> > Below is the updated device interface:
> > 
> > Currently, there are two regions of this device: 1) CONFIG_REGION
> > (VFIO_VHOST_CONFIG_REGION_INDEX), which can be used to setup the
> > device; 2) NOTIFY_REGION (VFIO_VHOST_NOTIFY_REGION_INDEX), which
> > can be used to notify the device.
> > 
> > 1. CONFIG_REGION
> > 
> > The region described by CONFIG_REGION is the main control interface.
> > Messages will be written to or read from this region.
> > 
> > The message type is determined by the `request` field in message
> > header. The message size is encoded in the message header too.
> > The message format looks like this:
> > 
> > struct vhost_vfio_op {
> > 	__u64 request;
> > 	__u32 flags;
> > 	/* Flag values: */
> >   #define VHOST_VFIO_NEED_REPLY 0x1 /* Whether need reply */
> > 	__u32 size;
> > 	union {
> > 		__u64 u64;
> > 		struct vhost_vring_state state;
> > 		struct vhost_vring_addr addr;
> > 	} payload;
> > };
> > 
> > The existing vhost-kernel ioctl cmds are reused as the message
> > requests in above structure.
> 
> 
> Still a comments like V1. What's the advantage of inventing a new protocol?

I'm trying to make it work in VFIO's way..

> I believe either of the following should be better:
> 
> - using vhost ioctl,  we can start from SET_VRING_KICK/SET_VRING_CALL and
> extend it with e.g notify region. The advantages is that all exist userspace
> program could be reused without modification (or minimal modification). And
> vhost API hides lots of details that is not necessary to be understood by
> application (e.g in the case of container).

Do you mean reusing vhost's ioctl on VFIO device fd directly,
or introducing another mdev driver (i.e. vhost_mdev instead of
using the existing vfio_mdev) for mdev device?

> 
> - using PCI layout, then you don't even need to re-invent notifiy region at
> all and we can pass-through them to guest.

Like what you said previously, virtio has transports other than PCI.
And it will look a bit odd when using transports other than PCI..

> 
> Personally, I prefer vhost ioctl.

+1

> 
> 
> > 
[...]
> > 
> > 3. VFIO interrupt ioctl API
> > 
> > VFIO interrupt ioctl API is used to setup device interrupts.
> > IRQ-bypass can also be supported.
> > 
> > Currently, the data path interrupt can be configured via the
> > VFIO_VHOST_VQ_IRQ_INDEX with virtqueue's callfd.
> 
> 
> How about DMA API? Do you expect to use VFIO IOMMU API or using vhost
> SET_MEM_TABLE? VFIO IOMMU API is more generic for sure but with
> SET_MEM_TABLE DMA can be done at the level of parent device which means it
> can work for e.g the card with on-chip IOMMU.

Agree. In this RFC, it assumes userspace will use VFIO IOMMU API
to do the DMA programming. But like what you said, there could be
a problem when using cards with on-chip IOMMU.

> 
> And what's the plan for vIOMMU?

As this RFC assumes userspace will use VFIO IOMMU API, userspace
just needs to follow the same way like what vfio-pci device does
in QEMU to support vIOMMU.

> 
> 
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> >   drivers/vhost/Makefile     |   2 +
> >   drivers/vhost/vdpa.c       | 770 +++++++++++++++++++++++++++++++++++++
> >   include/linux/vdpa_mdev.h  |  72 ++++
> >   include/uapi/linux/vfio.h  |  19 +
> >   include/uapi/linux/vhost.h |  25 ++
> >   5 files changed, 888 insertions(+)
> >   create mode 100644 drivers/vhost/vdpa.c
> >   create mode 100644 include/linux/vdpa_mdev.h
> 
> 
> We probably need some sample parent device implementation. It could be a
> software datapath like e.g we can start from virtio-net device in guest or a
> vhost/tap on host.

Yeah, something like this would be interesting!

Thanks,
Tiwei

> 
> Thanks
> 
> 
> > 

^ permalink raw reply

* [PATCH bpf-next] selftests/bpf: fix test_reuseport_array on s390
From: Ilya Leoshkevich @ 2019-07-03 11:50 UTC (permalink / raw)
  To: bpf, netdev; +Cc: Ilya Leoshkevich

Fix endianness issue: passing a pointer to 64-bit fd as a 32-bit key
does not work on big-endian architectures. So cast fd to 32-bits when
necessary.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/testing/selftests/bpf/test_maps.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index a3fbc571280a..5443b9bd75ed 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1418,7 +1418,7 @@ static void test_map_wronly(void)
 	assert(bpf_map_get_next_key(fd, &key, &value) == -1 && errno == EPERM);
 }
 
-static void prepare_reuseport_grp(int type, int map_fd,
+static void prepare_reuseport_grp(int type, int map_fd, size_t map_elem_size,
 				  __s64 *fds64, __u64 *sk_cookies,
 				  unsigned int n)
 {
@@ -1428,6 +1428,8 @@ static void prepare_reuseport_grp(int type, int map_fd,
 	const int optval = 1;
 	unsigned int i;
 	u64 sk_cookie;
+	void *value;
+	__s32 fd32;
 	__s64 fd64;
 	int err;
 
@@ -1449,8 +1451,14 @@ static void prepare_reuseport_grp(int type, int map_fd,
 		      "err:%d errno:%d\n", err, errno);
 
 		/* reuseport_array does not allow unbound sk */
-		err = bpf_map_update_elem(map_fd, &index0, &fd64,
-					  BPF_ANY);
+		if (map_elem_size == sizeof(__u64))
+			value = &fd64;
+		else {
+			assert(map_elem_size == sizeof(__u32));
+			fd32 = (__s32)fd64;
+			value = &fd32;
+		}
+		err = bpf_map_update_elem(map_fd, &index0, value, BPF_ANY);
 		CHECK(err != -1 || errno != EINVAL,
 		      "reuseport array update unbound sk",
 		      "sock_type:%d err:%d errno:%d\n",
@@ -1478,7 +1486,7 @@ static void prepare_reuseport_grp(int type, int map_fd,
 			 * reuseport_array does not allow
 			 * non-listening tcp sk.
 			 */
-			err = bpf_map_update_elem(map_fd, &index0, &fd64,
+			err = bpf_map_update_elem(map_fd, &index0, value,
 						  BPF_ANY);
 			CHECK(err != -1 || errno != EINVAL,
 			      "reuseport array update non-listening sk",
@@ -1541,7 +1549,7 @@ static void test_reuseport_array(void)
 	for (t = 0; t < ARRAY_SIZE(types); t++) {
 		type = types[t];
 
-		prepare_reuseport_grp(type, map_fd, grpa_fds64,
+		prepare_reuseport_grp(type, map_fd, sizeof(__u64), grpa_fds64,
 				      grpa_cookies, ARRAY_SIZE(grpa_fds64));
 
 		/* Test BPF_* update flags */
@@ -1649,7 +1657,8 @@ static void test_reuseport_array(void)
 				sizeof(__u32), sizeof(__u32), array_size, 0);
 	CHECK(map_fd == -1, "reuseport array create",
 	      "map_fd:%d, errno:%d\n", map_fd, errno);
-	prepare_reuseport_grp(SOCK_STREAM, map_fd, &fd64, &sk_cookie, 1);
+	prepare_reuseport_grp(SOCK_STREAM, map_fd, sizeof(__u32), &fd64,
+			      &sk_cookie, 1);
 	fd = fd64;
 	err = bpf_map_update_elem(map_fd, &index3, &fd, BPF_NOEXIST);
 	CHECK(err == -1, "reuseport array update 32 bit fd",
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Jiri Pirko @ 2019-07-03 11:49 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: David Miller, netdev, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
	linux-kernel
In-Reply-To: <cb614bebee1686293127194e8f7ced72955c7c7f.1562067622.git.mkubecek@suse.cz>

Tue, Jul 02, 2019 at 01:50:09PM CEST, mkubecek@suse.cz wrote:
>The ethtool netlink code uses common framework for passing arbitrary
>length bit sets to allow future extensions. A bitset can be a list (only
>one bitmap) or can consist of value and mask pair (used e.g. when client
>want to modify only some bits). A bitset can use one of two formats:
>verbose (bit by bit) or compact.
>
>Verbose format consists of bitset size (number of bits), list flag and
>an array of bit nests, telling which bits are part of the list or which
>bits are in the mask and which of them are to be set. In requests, bits
>can be identified by index (position) or by name. In replies, kernel
>provides both index and name. Verbose format is suitable for "one shot"
>applications like standard ethtool command as it avoids the need to
>either keep bit names (e.g. link modes) in sync with kernel or having to
>add an extra roundtrip for string set request (e.g. for private flags).
>
>Compact format uses one (list) or two (value/mask) arrays of 32-bit
>words to store the bitmap(s). It is more suitable for long running
>applications (ethtool in monitor mode or network management daemons)
>which can retrieve the names once and then pass only compact bitmaps to
>save space.
>
>Userspace requests can use either format and ETHTOOL_RF_COMPACT flag in
>request header tells kernel which format to use in reply. Notifications
>always use compact format.
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>---
> Documentation/networking/ethtool-netlink.txt |  61 ++
> include/uapi/linux/ethtool_netlink.h         |  35 ++
> net/ethtool/Makefile                         |   2 +-
> net/ethtool/bitset.c                         | 606 +++++++++++++++++++
> net/ethtool/bitset.h                         |  40 ++
> net/ethtool/netlink.h                        |   9 +
> 6 files changed, 752 insertions(+), 1 deletion(-)
> create mode 100644 net/ethtool/bitset.c
> create mode 100644 net/ethtool/bitset.h
>
>diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
>index 97c369aa290b..4636682c551f 100644
>--- a/Documentation/networking/ethtool-netlink.txt
>+++ b/Documentation/networking/ethtool-netlink.txt
>@@ -73,6 +73,67 @@ set, the behaviour is the same as (or closer to) the behaviour before it was
> introduced.
> 
> 
>+Bit sets
>+--------
>+
>+For short bitmaps of (reasonably) fixed length, standard NLA_BITFIELD32 type
>+is used. For arbitrary length bitmaps, ethtool netlink uses a nested attribute
>+with contents of one of two forms: compact (two binary bitmaps representing
>+bit values and mask of affected bits) and bit-by-bit (list of bits identified
>+by either index or name).
>+
>+Compact form: nested (bitset) atrribute contents:
>+
>+    ETHTOOL_A_BITSET_LIST	(flag)		no mask, only a list
>+    ETHTOOL_A_BITSET_SIZE	(u32)		number of significant bits
>+    ETHTOOL_A_BITSET_VALUE	(binary)	bitmap of bit values
>+    ETHTOOL_A_BITSET_MASK	(binary)	bitmap of valid bits
>+
>+Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
>+to a multiple of 32 bits. They consist of 32-bit words in host byte order,

Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
nested array of NLA_BITFIELD32 instead?


>+words ordered from least significant to most significant (i.e. the same way as
>+bitmaps are passed with ioctl interface).
>+
>+For compact form, ETHTOOL_A_BITSET_SIZE and ETHTOOL_A_BITSET_VALUE are
>+mandatory.  Similar to BITFIELD32, a compact form bit set requests to set bits

Double space^^


>+in the mask to 1 (if the bit is set in value) or 0 (if not) and preserve the
>+rest. If ETHTOOL_A_BITSET_LIST is present, there is no mask and bitset
>+represents a simple list of bits.

Okay, that is a bit confusing. Why not to rename to something like:
ETHTOOL_A_BITSET_NO_MASK (flag)
?


>+
>+Kernel bit set length may differ from userspace length if older application is
>+used on newer kernel or vice versa. If userspace bitmap is longer, an error is
>+issued only if the request actually tries to set values of some bits not
>+recognized by kernel.
>+
>+Bit-by-bit form: nested (bitset) attribute contents:
>+
>+    ETHTOOL_A_BITSET_LIST	(flag)		no mask, only a list
>+    ETHTOOL_A_BITSET_SIZE	(u32)		number of significant bits
>+    ETHTOOL_A_BITSET_BIT	(nested)	array of bits
>+	ETHTOOL_A_BITSET_BIT+   (nested)	one bit
>+	    ETHTOOL_A_BIT_INDEX	(u32)		bit index (0 for LSB)
>+	    ETHTOOL_A_BIT_NAME	(string)	bit name
>+	    ETHTOOL_A_BIT_VALUE	(flag)		present if bit is set
>+
>+Bit size is optional for bit-by-bit form. ETHTOOL_A_BITSET_BITS nest can only
>+contain ETHTOOL_A_BITS_BIT attributes but there can be an arbitrary number of
>+them.  A bit may be identified by its index or by its name. When used in
>+requests, listed bits are set to 0 or 1 according to ETHTOOL_A_BIT_VALUE, the
>+rest is preserved. A request fails if index exceeds kernel bit length or if
>+name is not recognized.
>+
>+When ETHTOOL_A_BITSET_LIST flag is present, bitset is interpreted as a simple
>+bit list. ETHTOOL_A_BIT_VALUE attributes are not used in such case. Bit list
>+represents a bitmap with listed bits set and the rest zero.
>+
>+In requests, application can use either form. Form used by kernel in reply is
>+determined by a flag in flags field of request header. Semantics of value and
>+mask depends on the attribute. General idea is that flags control request
>+processing, info_mask control which parts of the information are returned in
>+"get" request and index identifies a particular subcommand or an object to
>+which the request applies.

This is quite complex and confusing. Having the same API for 2 APIs is
odd. The API should be crystal clear, easy to use.

Why can't you have 2 commands, one working with bit arrays only, one
working with strings? Something like:
X_GET
   ETHTOOL_A_BITS (nested)
      ETHTOOL_A_BIT_ARRAY (BITFIELD32)
X_NAMES_GET
   ETHTOOL_A_BIT_NAMES (nested)
	ETHTOOL_A_BIT_INDEX
	ETHTOOL_A_BIT_NAME

For set, you can also have multiple cmds:
X_SET  - to set many at once, by bit index
   ETHTOOL_A_BITS (nested)
      ETHTOOL_A_BIT_ARRAY (BITFIELD32)
X_ONE_SET   - to set one, by bit index
   ETHTOOL_A_BIT_INDEX
   ETHTOOL_A_BIT_VALUE
X_ONE_SET   - to set one, by name
   ETHTOOL_A_BIT_NAME
   ETHTOOL_A_BIT_VALUE


[...]

^ permalink raw reply

* Re: [PATCH] bpf, libbpf: Smatch: Fix potential NULL pointer dereference
From: Leo Yan @ 2019-07-03 11:46 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
	netdev, bpf, linux-kernel, Dan Carpenter
In-Reply-To: <b834fba1-5b2c-4406-8275-1cf8383655e3@iogearbox.net>

On Wed, Jul 03, 2019 at 12:23:05PM +0200, Daniel Borkmann wrote:
> On 07/02/2019 12:25 PM, Leo Yan wrote:
> > Based on the following report from Smatch, fix the potential
> > NULL pointer dereference check.
> > 
> >   tools/lib/bpf/libbpf.c:3493
> >   bpf_prog_load_xattr() warn: variable dereferenced before check 'attr'
> >   (see line 3483)
> > 
> > 3479 int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> > 3480                         struct bpf_object **pobj, int *prog_fd)
> > 3481 {
> > 3482         struct bpf_object_open_attr open_attr = {
> > 3483                 .file           = attr->file,
> > 3484                 .prog_type      = attr->prog_type,
> >                                        ^^^^^^
> > 3485         };
> > 
> > At the head of function, it directly access 'attr' without checking if
> > it's NULL pointer.  This patch moves the values assignment after
> > validating 'attr' and 'attr->file'.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/lib/bpf/libbpf.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 197b574406b3..809b633fa3d9 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3479,10 +3479,7 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
> >  int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> >  			struct bpf_object **pobj, int *prog_fd)
> >  {
> > -	struct bpf_object_open_attr open_attr = {
> > -		.file		= attr->file,
> > -		.prog_type	= attr->prog_type,
> > -	};
> 
> Applied, thanks! Fyi, I retained the zeroing of open_attr as otherwise if we ever
> extend struct bpf_object_open_attr in future, we'll easily miss this and pass in
> garbage to bpf_object__open_xattr().

Thanks for the info, Daniel.

I checked the link [1] and thanks for the improvement when applied this
patch.

Thanks,
Leo Yan

[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=33bae185f74d49a0d7b1bfaafb8e959efce0f243

^ permalink raw reply

* Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-07-03 11:41 UTC (permalink / raw)
  To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20190702234629.zgjpu664dr6jgawc@salvia>

On Wed, Jul 03, 2019 at 01:46:29AM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jun 28, 2019 at 08:49:34AM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> > 
> > This patch provide a meta to get the bridge vlan proto
> > 
> > nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
> 
> Applied, thanks.

Tossing this:

net/netfilter/nft_meta.c: In function ‘nft_meta_get_eval’:
net/netfilter/nft_meta.c:255:31: error: ‘struct net_bridge’ has no
member named ‘vlan_proto’
    nft_reg_store16(dest, p->br->vlan_proto);
                               ^~


^ permalink raw reply

* [PATCH v2] rt2x00: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-07-03 11:39 UTC (permalink / raw)
  To: Stanislaw Gruszka, Helmut Schaa, Kalle Valo
  Cc: David S. Miller, linux-wireless, netdev
In-Reply-To: <20190703065631.GA28822@kroah.com>


When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Because we don't need to save the individual debugfs files and
directories, remove the local storage of them and just remove the entire
debugfs directory in a single call, making things a lot simpler.

Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Helmut Schaa <helmut.schaa@googlemail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: - rebase against wireless-drivers-next
    - clean up the debugfs "blob" creation logic a bit more, no need to
      care about that return value.

 .../net/wireless/ralink/rt2x00/rt2x00debug.c  | 136 +++++-------------
 1 file changed, 35 insertions(+), 101 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00debug.c b/drivers/net/wireless/ralink/rt2x00/rt2x00debug.c
index ef5f51512212..4d4e3888ef20 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00debug.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00debug.c
@@ -65,26 +65,6 @@ struct rt2x00debug_intf {
 	 *     - crypto stats file
 	 */
 	struct dentry *driver_folder;
-	struct dentry *driver_entry;
-	struct dentry *chipset_entry;
-	struct dentry *dev_flags;
-	struct dentry *cap_flags;
-	struct dentry *restart_hw;
-	struct dentry *register_folder;
-	struct dentry *csr_off_entry;
-	struct dentry *csr_val_entry;
-	struct dentry *eeprom_off_entry;
-	struct dentry *eeprom_val_entry;
-	struct dentry *bbp_off_entry;
-	struct dentry *bbp_val_entry;
-	struct dentry *rf_off_entry;
-	struct dentry *rf_val_entry;
-	struct dentry *rfcsr_off_entry;
-	struct dentry *rfcsr_val_entry;
-	struct dentry *queue_folder;
-	struct dentry *queue_frame_dump_entry;
-	struct dentry *queue_stats_entry;
-	struct dentry *crypto_stats_entry;
 
 	/*
 	 * The frame dump file only allows a single reader,
@@ -596,39 +576,34 @@ static const struct file_operations rt2x00debug_restart_hw = {
 	.llseek = generic_file_llseek,
 };
 
-static struct dentry *rt2x00debug_create_file_driver(const char *name,
-						     struct rt2x00debug_intf
-						     *intf,
-						     struct debugfs_blob_wrapper
-						     *blob)
+static void rt2x00debug_create_file_driver(const char *name,
+					   struct rt2x00debug_intf *intf,
+					   struct debugfs_blob_wrapper *blob)
 {
 	char *data;
 
 	data = kzalloc(3 * MAX_LINE_LENGTH, GFP_KERNEL);
 	if (!data)
-		return NULL;
+		return;
 
 	blob->data = data;
 	data += sprintf(data, "driver:\t%s\n", intf->rt2x00dev->ops->name);
 	data += sprintf(data, "version:\t%s\n", DRV_VERSION);
 	blob->size = strlen(blob->data);
 
-	return debugfs_create_blob(name, 0400, intf->driver_folder, blob);
+	debugfs_create_blob(name, 0400, intf->driver_folder, blob);
 }
 
-static struct dentry *rt2x00debug_create_file_chipset(const char *name,
-						      struct rt2x00debug_intf
-						      *intf,
-						      struct
-						      debugfs_blob_wrapper
-						      *blob)
+static void rt2x00debug_create_file_chipset(const char *name,
+					    struct rt2x00debug_intf *intf,
+					    struct debugfs_blob_wrapper *blob)
 {
 	const struct rt2x00debug *debug = intf->debug;
 	char *data;
 
 	data = kzalloc(9 * MAX_LINE_LENGTH, GFP_KERNEL);
 	if (!data)
-		return NULL;
+		return;
 
 	blob->data = data;
 	data += sprintf(data, "rt chip:\t%04x\n", intf->rt2x00dev->chip.rt);
@@ -654,13 +629,15 @@ static struct dentry *rt2x00debug_create_file_chipset(const char *name,
 
 	blob->size = strlen(blob->data);
 
-	return debugfs_create_blob(name, 0400, intf->driver_folder, blob);
+	debugfs_create_blob(name, 0400, intf->driver_folder, blob);
 }
 
 void rt2x00debug_register(struct rt2x00_dev *rt2x00dev)
 {
 	const struct rt2x00debug *debug = rt2x00dev->ops->debugfs;
 	struct rt2x00debug_intf *intf;
+	struct dentry *queue_folder;
+	struct dentry *register_folder;
 
 	intf = kzalloc(sizeof(struct rt2x00debug_intf), GFP_KERNEL);
 	if (!intf) {
@@ -676,43 +653,27 @@ void rt2x00debug_register(struct rt2x00_dev *rt2x00dev)
 	    debugfs_create_dir(intf->rt2x00dev->ops->name,
 			       rt2x00dev->hw->wiphy->debugfsdir);
 
-	intf->driver_entry =
-	    rt2x00debug_create_file_driver("driver", intf, &intf->driver_blob);
+	rt2x00debug_create_file_driver("driver", intf, &intf->driver_blob);
+	rt2x00debug_create_file_chipset("chipset", intf, &intf->chipset_blob);
+	debugfs_create_file("dev_flags", 0400, intf->driver_folder, intf,
+			    &rt2x00debug_fop_dev_flags);
+	debugfs_create_file("cap_flags", 0400, intf->driver_folder, intf,
+			    &rt2x00debug_fop_cap_flags);
+	debugfs_create_file("restart_hw", 0200, intf->driver_folder, intf,
+			    &rt2x00debug_restart_hw);
 
-	intf->chipset_entry =
-	    rt2x00debug_create_file_chipset("chipset",
-					    intf, &intf->chipset_blob);
-
-	intf->dev_flags = debugfs_create_file("dev_flags", 0400,
-					      intf->driver_folder, intf,
-					      &rt2x00debug_fop_dev_flags);
-
-	intf->cap_flags = debugfs_create_file("cap_flags", 0400,
-					      intf->driver_folder, intf,
-					      &rt2x00debug_fop_cap_flags);
-
-	intf->restart_hw = debugfs_create_file("restart_hw", 0200,
-					       intf->driver_folder, intf,
-					       &rt2x00debug_restart_hw);
-
-	intf->register_folder =
-	    debugfs_create_dir("register", intf->driver_folder);
+	register_folder = debugfs_create_dir("register", intf->driver_folder);
 
 #define RT2X00DEBUGFS_CREATE_REGISTER_ENTRY(__intf, __name)		\
 ({									\
 	if (debug->__name.read) {					\
-		(__intf)->__name##_off_entry =				\
-			debugfs_create_u32(__stringify(__name) "_offset", \
-					   0600,			\
-					   (__intf)->register_folder,	\
-					   &(__intf)->offset_##__name);	\
+		debugfs_create_u32(__stringify(__name) "_offset", 0600,	\
+				   register_folder,			\
+				   &(__intf)->offset_##__name);		\
 									\
-		(__intf)->__name##_val_entry =				\
-			debugfs_create_file(__stringify(__name) "_value", \
-					    0600,			\
-					    (__intf)->register_folder,	\
-					    (__intf),			\
-					    &rt2x00debug_fop_##__name); \
+		debugfs_create_file(__stringify(__name) "_value", 0600,	\
+				    register_folder, (__intf),		\
+				    &rt2x00debug_fop_##__name);		\
 	}								\
 })
 
@@ -724,26 +685,21 @@ void rt2x00debug_register(struct rt2x00_dev *rt2x00dev)
 
 #undef RT2X00DEBUGFS_CREATE_REGISTER_ENTRY
 
-	intf->queue_folder =
-	    debugfs_create_dir("queue", intf->driver_folder);
+	queue_folder = debugfs_create_dir("queue", intf->driver_folder);
 
-	intf->queue_frame_dump_entry =
-		debugfs_create_file("dump", 0400, intf->queue_folder,
-				    intf, &rt2x00debug_fop_queue_dump);
+	debugfs_create_file("dump", 0400, queue_folder, intf,
+			    &rt2x00debug_fop_queue_dump);
 
 	skb_queue_head_init(&intf->frame_dump_skbqueue);
 	init_waitqueue_head(&intf->frame_dump_waitqueue);
 
-	intf->queue_stats_entry =
-		debugfs_create_file("queue", 0400, intf->queue_folder,
-				    intf, &rt2x00debug_fop_queue_stats);
+	debugfs_create_file("queue", 0400, queue_folder, intf,
+			    &rt2x00debug_fop_queue_stats);
 
 #ifdef CONFIG_RT2X00_LIB_CRYPTO
 	if (rt2x00_has_cap_hw_crypto(rt2x00dev))
-		intf->crypto_stats_entry =
-			debugfs_create_file("crypto", 0444, intf->queue_folder,
-					    intf,
-					    &rt2x00debug_fop_crypto_stats);
+		debugfs_create_file("crypto", 0444, queue_folder, intf,
+				    &rt2x00debug_fop_crypto_stats);
 #endif
 
 	return;
@@ -758,29 +714,7 @@ void rt2x00debug_deregister(struct rt2x00_dev *rt2x00dev)
 
 	skb_queue_purge(&intf->frame_dump_skbqueue);
 
-#ifdef CONFIG_RT2X00_LIB_CRYPTO
-	debugfs_remove(intf->crypto_stats_entry);
-#endif
-	debugfs_remove(intf->queue_stats_entry);
-	debugfs_remove(intf->queue_frame_dump_entry);
-	debugfs_remove(intf->queue_folder);
-	debugfs_remove(intf->rfcsr_val_entry);
-	debugfs_remove(intf->rfcsr_off_entry);
-	debugfs_remove(intf->rf_val_entry);
-	debugfs_remove(intf->rf_off_entry);
-	debugfs_remove(intf->bbp_val_entry);
-	debugfs_remove(intf->bbp_off_entry);
-	debugfs_remove(intf->eeprom_val_entry);
-	debugfs_remove(intf->eeprom_off_entry);
-	debugfs_remove(intf->csr_val_entry);
-	debugfs_remove(intf->csr_off_entry);
-	debugfs_remove(intf->register_folder);
-	debugfs_remove(intf->dev_flags);
-	debugfs_remove(intf->restart_hw);
-	debugfs_remove(intf->cap_flags);
-	debugfs_remove(intf->chipset_entry);
-	debugfs_remove(intf->driver_entry);
-	debugfs_remove(intf->driver_folder);
+	debugfs_remove_recursive(intf->driver_folder);
 	kfree(intf->chipset_blob.data);
 	kfree(intf->driver_blob.data);
 	kfree(intf);
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH] netfilter: nf_log: Replace a seq_printf() call by seq_puts() in seq_show()
From: Pablo Neira Ayuso @ 2019-07-03 11:38 UTC (permalink / raw)
  To: Markus Elfring
  Cc: netfilter-devel, coreteam, netdev, David S. Miller,
	Florian Westphal, Jozsef Kadlecsik, LKML, kernel-janitors
In-Reply-To: <c7d397c8-4f41-1831-505f-b3fbcc3663fb@web.de>

On Tue, Jul 02, 2019 at 08:11:53PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 Jul 2019 20:06:30 +0200
> 
> A string which did not contain a data format specification should be put
> into a sequence. Thus use the corresponding function “seq_puts”.
> 
> This issue was detected by using the Coccinelle software.

Applied, thanks.

^ permalink raw reply

* [PATCH net] net: hns: add support for vlan TSO
From: Yonglong Liu @ 2019-07-03 11:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, linuxarm, salil.mehta, yisen.zhuang,
	shiju.jose

The hip07 chip support vlan TSO, this patch adds NETIF_F_TSO
and NETIF_F_TSO6 flags to vlan_features to improve the
performance after adding vlan to the net ports.

Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index fe879c0..2235dd5 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -2370,6 +2370,7 @@ static int hns_nic_dev_probe(struct platform_device *pdev)
 		ndev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 			NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_GSO |
 			NETIF_F_GRO | NETIF_F_TSO | NETIF_F_TSO6;
+		ndev->vlan_features |= NETIF_F_TSO | NETIF_F_TSO6;
 		ndev->max_mtu = MAC_MAX_MTU_V2 -
 				(ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
 		break;
-- 
2.8.1


^ permalink raw reply related

* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Michal Kubecek @ 2019-07-03 11:13 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
	linux-kernel
In-Reply-To: <20190703100435.GS2250@nanopsycho>

On Wed, Jul 03, 2019 at 12:04:35PM +0200, Jiri Pirko wrote:
> Tue, Jul 02, 2019 at 06:34:37PM CEST, mkubecek@suse.cz wrote:
> >On Tue, Jul 02, 2019 at 03:05:15PM +0200, Jiri Pirko wrote:
> >> Tue, Jul 02, 2019 at 01:50:04PM CEST, mkubecek@suse.cz wrote:
> >> >+
> >> >+	req_info->dev = dev;
> >> >+	ethnl_update_u32(&req_info->req_mask, tb[ETHTOOL_A_HEADER_INFOMASK]);
> >> >+	ethnl_update_u32(&req_info->global_flags, tb[ETHTOOL_A_HEADER_GFLAGS]);
> >> >+	ethnl_update_u32(&req_info->req_flags, tb[ETHTOOL_A_HEADER_RFLAGS]);
> >> 
> >> Just:
> >> 	req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK];
> >> 	...
> >> 
> >> Not sure what ethnl_update_u32() is good for, but it is not needed here.
> >
> >That would result in null pointer dereference if the attribute is
> >missing. So you would need at least
> >
> >	if (tb[ETHTOOL_A_HEADER_INFOMASK])
> >		req_info->req_mask = nla_get_u32(tb[ETHTOOL_A_HEADER_INFOMASK]);
> >	if (tb[ETHTOOL_A_HEADER_GFLAGS])
> >		req_info->global_flags =
> >			nla_get_u32(tb[ETHTOOL_A_HEADER_GFLAGS]);
> >	if (tb[ETHTOOL_A_HEADER_RFLAGS])
> >		req_info->req_flags = nla_get_u32(tb[ETHTOOL_A_HEADER_RFLAGS]);
> 
> Yeah, sure.
> 
> >
> >I don't think it looks better.
> 
> Better than hiding something inside a helper in my opinion - helper that
> is there for different reason moreover. Much easier to read the code
> and follow.

OK, I'll use nla_get_u32() directly here. With the change below, use of
ethnl_update_u32() would really look unnatural.

> >> >+/* The ethnl_update_* helpers set value pointed to by @dst to the value of
> >> >+ * netlink attribute @attr (if attr is not null). They return true if *dst
> >> >+ * value was changed, false if not.
> >> >+ */
> >> >+static inline bool ethnl_update_u32(u32 *dst, struct nlattr *attr)
> >> 
> >> I'm still not sure I'm convinced about these "update helpers" :)
> >
> >Just imagine what would e.g.
> >
> >	if (ethnl_update_u32(&data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
> >		mod = true;
> >	if (ethnl_update_u32(&data.rx_mini_pending,
> >			     tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
> >		mod = true;
> >	if (ethnl_update_u32(&data.rx_jumbo_pending,
> >			     tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
> >		mod = true;
> >	if (ethnl_update_u32(&data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
> >		mod = true;
> >	if (!mod)
> >		return 0;
> >
> >look like without them. And coalescing parameters would be much worse
> >(22 attributes / struct members).
> 
> No, I understand your motivation, don't get me wrong. I just wonder that
> no other netlink implementation need such mechanism. Maybe I'm not
> looking close enough. But if it does, should be rathe netlink helper.

I'll check some existing interfaces to see how they handle "set" type
requests.

> Regarding the example code you have here. It is prefered to store
> function result in a variable "if check" that variable. But in your,
> code, couldn't this be done without ifs?
> 
> 	bool mod = false;
> 
> 	ethnl_update_u32(&mod, &data.rx_pending, tb[ETHTOOL_A_RING_RX_PENDING]))
> 	ethnl_update_u32(&mod, &data.rx_mini_pending,
> 			 tb[ETHTOOL_A_RING_RX_MINI_PENDING]))
> 	ethnl_update_u32(&mod, &data.rx_jumbo_pending,
> 			 tb[ETHTOOL_A_RING_RX_JUMBO_PENDING]))
> 	ethnl_update_u32(&mod, &data.tx_pending, tb[ETHTOOL_A_RING_TX_PENDING]))
> 	
> 	if (!mod)
> 		return 0;

Ah, right. Somehow I completely missed the possibility that update
helper can use "set of leave as it is" logic instead of "set to true or
false". Thanks, I'll rewrite the update helpers to this style.

Michal

^ permalink raw reply

* bug: tpacket_snd can cause data corruption
From: Frank de Brabander @ 2019-07-03 11:07 UTC (permalink / raw)
  To: David S . Miller, Willem de Bruijn; +Cc: netdev, Frank de Brabander

In commit 5cd8d46e a fix was applied for data corruption in
tpacket_snd. A selftest was added in commit 358be656 which
validates this fix.

Unfortunately this bug still persists, although since this fix less
likely to trigger. This bug was initially observed using a PACKET_MMAP
application, but can also be seen by tweaking the kernel selftest.

By tweaking the selftest txring_overwrite.c to run
as an infinite loop, the data corruption will still trigger. It
seems to occur faster by generating interrupts (e.g. by plugging
in USB devices). Tested with kernel version 5.2-RC7.

Cause for this bug is still unclear.

Signed-off-by: Frank de Brabander <debrabander@gmail.com>
---
 tools/testing/selftests/net/txring_overwrite.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/txring_overwrite.c b/tools/testing/selftests/net/txring_overwrite.c
index fd8b1c6..3ee23e5 100644
--- a/tools/testing/selftests/net/txring_overwrite.c
+++ b/tools/testing/selftests/net/txring_overwrite.c
@@ -143,19 +143,22 @@ static int read_verify_pkt(int fdr, char payload_char)
 	int ret;
 
 	ret = read(fdr, buf, sizeof(buf));
-	if (ret != sizeof(buf))
-		error(1, errno, "read");
+	if (ret != sizeof(buf)) {
+		//error(1, errno, "read");
+		printf("read error\n");
+		return 1;
+	}
 
 	if (buf[60] != payload_char) {
 		printf("wrong pattern: 0x%x != 0x%x\n", buf[60], payload_char);
 		return 1;
 	}
 
-	printf("read: %c (0x%x)\n", buf[60], buf[60]);
+	//printf("read: %c (0x%x)\n", buf[60], buf[60]);
 	return 0;
 }
 
-int main(int argc, char **argv)
+void run_test(void)
 {
 	const char payload_patterns[] = "ab";
 	char *ring;
@@ -177,3 +180,10 @@ int main(int argc, char **argv)
 
 	return ret;
 }
+
+int main(int argc, char **argv)
+{
+	while (true) {
+		run_test();
+	}
+}
-- 
2.7.4


^ permalink raw reply related


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