Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] [PATCH net-next] tun: fix multiqueue rx
From: Jason Wang @ 2018-11-16  6:29 UTC (permalink / raw)
  To: Matthew Cover, davem, brouer, mst, edumazet, sd, netdev,
	matthew.cover
In-Reply-To: <20181116041016.39852-1-matthew.cover@stackpath.com>


On 2018/11/16 下午12:10, Matthew Cover wrote:
> When writing packets to a descriptor associated with a combined queue, the
> packets should end up on that queue.
>
> Before this change all packets written to any descriptor associated with a
> tap interface end up on rx-0, even when the descriptor is associated with a
> different queue.
>
> The rx traffic can be generated by either of the following.
>    1. a simple tap program which spins up multiple queues and writes packets
>       to each of the file descriptors
>    2. tx from a qemu vm with a tap multiqueue netdev
>
> The queue for rx traffic can be observed by either of the following (done
> on the hypervisor in the qemu case).
>    1. a simple netmap program which opens and reads from per-queue
>       descriptors
>    2. configuring RPS and doing per-cpu captures with rxtxcpu
>
> Alternatively, if you printk() the return value of skb_get_rx_queue() just
> before each instance of netif_receive_skb() in tun.c, you will get 65535
> for every skb.
>
> Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes
> the association between descriptor and rx queue.
>
> Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
> ---
>   drivers/net/tun.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index a65779c6d72f..4e306ff3501c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
>   
>   	if (!rx_batched || (!more && skb_queue_empty(queue))) {
>   		local_bh_disable();
> +		skb_record_rx_queue(skb, tfile->queue_index);
>   		netif_receive_skb(skb);
>   		local_bh_enable();
>   		return;
> @@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
>   		struct sk_buff *nskb;
>   
>   		local_bh_disable();
> -		while ((nskb = __skb_dequeue(&process_queue)))
> +		while ((nskb = __skb_dequeue(&process_queue))) {
> +			skb_record_rx_queue(nskb, tfile->queue_index);
>   			netif_receive_skb(nskb);
> +		}
> +		skb_record_rx_queue(skb, tfile->queue_index);
>   		netif_receive_skb(skb);
>   		local_bh_enable();
>   	}


Thanks for the fix. Actually, there's another path which needs to be 
fixed as well in tun_xdp_one(). This path is used for vhost to pass a 
batched of packets.

^ permalink raw reply

* Re: [RFC] Discuss about an new idea "Vsock over Virtio-net"
From: Jason Wang @ 2018-11-16  6:35 UTC (permalink / raw)
  To: jiangyiwen, stefanha, stefanha, mst; +Cc: netdev, kvm, virtualization
In-Reply-To: <5BEE2C30.1030007@huawei.com>


On 2018/11/16 上午10:32, jiangyiwen wrote:
>>>>>> Note, if we've negotiated the feature, virtio-net driver must not use register_netdev to register it to network core. This can avoid lots of confusion.
>>>>> Hi Jason,
>>>>>
>>>>> You mean we should not register netdev if use vsock, and in order to
>>>>> avoid confusion, then I think whether we should keep vsock and export
>>>>> some virtio-net's functions that can be shared. In this way, first, vsock
>>>>> may keep existing architecture and will not affect virtio-net.
>>>> At least it needs new feature bits which will be a functional duplication of virtio-net (e.g mrg_rxbuf).
>>> Hi Jason,
>>>
>>> Actually I mean only use some shared function to make vsock support these
>>> features, in that way, guest see the device is still vsock device instead of
>>> virtio-net device, in addition, it can have less codes and easier to be
>>> compatible with old vsock version.
>> Yes, I think we're talking about same thing. Both of us want to share codes. What you want is to export and share some common helpers between virtio-net and vsock. What I meant is to e.g probe vsock device and merge vsock specific codes into virtio-net driver. I agree it's not a small project. We can start from e.g patches that try to share the codes. This could also give us inspiration for how to unify them.
>>
>>
> Then we have two ways to implement it.
> 1. For Guest, it is a virtio-net device(VIRTIO_ID_NET),
> use a feature bit(e.g VIRTIO_NET_F_VSOCK) to distinguish
> vsock special device and other virtio-net device. For old
> vsock device, it still use old driver to drive it.
>
> 2. For Guest, it is a virtio-vsock device(VIRTIO_ID_VSOCK),
> use virtio device id to distinguish them, it will integrate
> old driver, but it may be more complicated, because it needs
> to consider the compatibility with old vsock device.


This looks even better, btw what compatibility do you mean here? Looking 
at vsock driver, it does not have vosck specific feature.

Thanks

^ permalink raw reply

* Re: [PATCH net] sctp: not allow to set asoc prsctp_enable by sockopt
From: Xin Long @ 2018-11-16  6:42 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <20181115172221.GA3601@localhost.localdomain>

On Fri, Nov 16, 2018 at 2:22 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Thu, Nov 15, 2018 at 07:14:28PM +0800, Xin Long wrote:
> > As rfc7496#section4.5 says about SCTP_PR_SUPPORTED:
> >
> >    This socket option allows the enabling or disabling of the
> >    negotiation of PR-SCTP support for future associations.  For existing
> >    associations, it allows one to query whether or not PR-SCTP support
> >    was negotiated on a particular association.
> >
> > It means only sctp sock's prsctp_enable can be set.
> >
> > Note that for the limitation of SCTP_{CURRENT|ALL}_ASSOC, we will
> > add it when introducing SCTP_{FUTURE|CURRENT|ALL}_ASSOC for linux
> > sctp in another patchset.
> >
> > Fixes: 28aa4c26fce2 ("sctp: add SCTP_PR_SUPPORTED on sctp sockopt")
> > Reported-by: Ying Xu <yinxu@redhat.com>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  net/sctp/socket.c | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 739f3e5..e9b8232 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -3940,7 +3940,6 @@ static int sctp_setsockopt_pr_supported(struct sock *sk,
> >                                       unsigned int optlen)
> >  {
> >       struct sctp_assoc_value params;
> > -     struct sctp_association *asoc;
> >       int retval = -EINVAL;
> >
> >       if (optlen != sizeof(params))
> > @@ -3951,16 +3950,10 @@ static int sctp_setsockopt_pr_supported(struct sock *sk,
> >               goto out;
> >       }
> >
> > -     asoc = sctp_id2assoc(sk, params.assoc_id);
> > -     if (asoc) {
> > -             asoc->prsctp_enable = !!params.assoc_value;
> > -     } else if (!params.assoc_id) {
> > -             struct sctp_sock *sp = sctp_sk(sk);
> > -
> > -             sp->ep->prsctp_enable = !!params.assoc_value;
> > -     } else {
> > +     if (sctp_style(sk, UDP) && sctp_id2assoc(sk, params.assoc_id))
I got this semantic from BSD's SCTP_PR_SUPPORTED sockopt:
                        SCTP_FIND_STCB(inp, stcb, av->assoc_id);

                        if (stcb) {
                                SCTP_LTRACE_ERR_RET(...);
                                error = EINVAL;
                                SCTP_TCB_UNLOCK(stcb);
                        } else {
                                ...
                        }

>
> This would allow using a non-existent assoc id on UDP-style sockets to
> set it at the socket, which is not expected. It should be more like:
>
> +       if (sctp_style(sk, UDP) && params.assoc_id)
This way is more strict, but it seems reasonable.

When a user sets params.assoc_id for UDP type socket, it should be
thought as he WANTs to apply this on assoc, which is not allowed here.

^ permalink raw reply

* Re: [PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC
From: biao huang @ 2018-11-16  6:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: davem, honghui.zhang, yt.shen, liguo.zhang, mark.rutland,
	sean.wang, nelson.chang, matthias.bgg, netdev, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, joabreu, andrew
In-Reply-To: <20181112162204.GA8910@bogus>

Dear Rob,

Thanks for your comments.

On Mon, 2018-11-12 at 10:22 -0600, Rob Herring wrote:
> On Mon, Oct 29, 2018 at 11:04:54AM +0800, Biao Huang wrote:
> > The commit adds the device tree binding documentation for the MediaTek DWMAC
> > found on MediaTek MT2712.
> > 
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >  .../devicetree/bindings/net/mediatek-dwmac.txt     |   49 ++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > new file mode 100644
> > index 0000000..8baf0e4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > @@ -0,0 +1,49 @@
> > +MediaTek DWMAC glue layer controller
> > +
> > +This file documents platform glue layer for stmmac.
> > +Please see stmmac.txt for the other unchanged properties.
> > +
> > +The device node has following properties.
> > +
> > +Required properties:
> > +- compatible:  Should be "mediatek,mt2712-gmac" for MT2712 SoC
> > +- reg:  Address and length of the register set for the device
> > +- interrupts:  Should contain the MAC interrupts
> > +- interrupt-names: Should contain a list of interrupt names corresponding to
> > +	the interrupts in the interrupts property, if available.
> > +	Should be "macirq" for the main MAC IRQ
> > +- clocks: Must contain a phandle for each entry in clock-names.
> > +- clock-names: the name of the clock listed in the clocks property. These are
> > +	"axi", "apb", "mac_ext", "mac_parent", "ptp_ref", "ptp_parent", "ptp_top"
> > +	for MT2712 SoC
> 
> Clocks should represent the physical clocks connected to a block. Parent 
> clocks are not in that category.
There are two categories of physical clocks related to MAC.
1. axi/apb/ptp_top, these clks are from clk gates, so only on/off is
needed.
2. mac_ext/ptp_ref, these clks are from mux. If different source is
selected, the clk frequecy will be different. So it's necessary to set a
proper parent for them, i.e. mac_parent/ptp_parent.

How should I handle parent clocks if not defined here? Any examples I
can refer to?
> 
> > +- mac-address: See ethernet.txt in the same directory
> > +- phy-mode: See ethernet.txt in the same directory
> > +
> > +Example:
> > +
> > +	eth: ethernet@1101c000 {
> > +		compatible = "mediatek,mt2712-gmac";
> > +		reg = <0 0x1101c000 0 0x1300>;
> > +		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-names = "macirq";
> > +		phy-mode ="rgmii-id";
> > +		mac-address = [00 55 7b b5 7d f7];
> > +		clock-names = "axi",
> > +			      "apb",
> > +			      "mac_ext",
> > +			      "mac_parent",
> > +			      "ptp_ref",
> > +			      "ptp_parent",
> > +			      "ptp_top";
> > +		clocks = <&pericfg CLK_PERI_GMAC>,
> > +			 <&pericfg CLK_PERI_GMAC_PCLK>,
> > +			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > +			 <&topckgen CLK_TOP_ETHERPLL_125M>,
> > +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +			 <&topckgen CLK_TOP_APLL1_D3>,
> > +			 <&topckgen CLK_TOP_APLL1>;
> > +		snps,txpbl = <32>;
> > +		snps,rxpbl = <32>;
> > +		snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> > +		snps,reset-active-low;
> > +	};
> > -- 
> > 1.7.9.5
> > 
> 
Best Regards!
Biao

^ permalink raw reply

* [PATCH] [PATCH net-next] tun: fix multiqueue rx
From: Matthew Cover @ 2018-11-16  7:00 UTC (permalink / raw)
  To: davem, jasowang, brouer, mst, edumazet, sd, netdev, matthew.cover
In-Reply-To: <20181116041016.39852-1-matthew.cover@stackpath.com>

When writing packets to a descriptor associated with a combined queue, the
packets should end up on that queue.

Before this change all packets written to any descriptor associated with a
tap interface end up on rx-0, even when the descriptor is associated with a
different queue.

The rx traffic can be generated by either of the following.
  1. a simple tap program which spins up multiple queues and writes packets
     to each of the file descriptors
  2. tx from a qemu vm with a tap multiqueue netdev

The queue for rx traffic can be observed by either of the following (done
on the hypervisor in the qemu case).
  1. a simple netmap program which opens and reads from per-queue
     descriptors
  2. configuring RPS and doing per-cpu captures with rxtxcpu

Alternatively, if you printk() the return value of skb_get_rx_queue() just
before each instance of netif_receive_skb() in tun.c, you will get 65535
for every skb.

Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes
the association between descriptor and rx queue.

Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
---
 drivers/net/tun.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index a65779c6d72f..ce8620f3ea5e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
 
 	if (!rx_batched || (!more && skb_queue_empty(queue))) {
 		local_bh_disable();
+		skb_record_rx_queue(skb, tfile->queue_index);
 		netif_receive_skb(skb);
 		local_bh_enable();
 		return;
@@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
 		struct sk_buff *nskb;
 
 		local_bh_disable();
-		while ((nskb = __skb_dequeue(&process_queue)))
+		while ((nskb = __skb_dequeue(&process_queue))) {
+			skb_record_rx_queue(nskb, tfile->queue_index);
 			netif_receive_skb(nskb);
+		}
+		skb_record_rx_queue(skb, tfile->queue_index);
 		netif_receive_skb(skb);
 		local_bh_enable();
 	}
@@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun,
 	    !tfile->detached)
 		rxhash = __skb_get_hash_symmetric(skb);
 
+	skb_record_rx_queue(skb, tfile->queue_index);
 	netif_receive_skb(skb);
 
 	stats = get_cpu_ptr(tun->pcpu_stats);
-- 
2.15.2 (Apple Git-101.1)

^ permalink raw reply related

* Re: [PATCHv2 net] ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf
From: Julian Anastasov @ 2018-11-16  7:10 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, netfilter-devel, David S. Miller, pablo,
	Hans Schillstrom, Simon Horman
In-Reply-To: <832d0eccc937ab122b337e627244533df3eb74a5.1542266070.git.lucien.xin@gmail.com>


	Hello,

On Thu, 15 Nov 2018, Xin Long wrote:

> ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> destinations when a net dev is going down. But it works only
> when the dst's dev is the same as the dev from the event.
> 
> Now with the same priority but late registration,
> ip_vs_dst_notifier is always called later than ipv6_dev_notf
> where the dst's dev is set to lo for NETDEV_DOWN event.
> 
> As the dst's dev lo is not the same as the dev from the event
> in ip_vs_dst_event, ip_vs_dst_notifier doesn't actually work.
> Also as these dst have to wait for dest_trash_timer to clean
> them up. It would cause some non-permanent kernel warnings:
> 
>   unregister_netdevice: waiting for br0 to become free. Usage count = 3
> 
> To fix it, call ip_vs_dst_notifier earlier than ipv6_dev_notf
> by increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> 
> Note that for ipv4 route fib_netdev_notifier doesn't set dst's
> dev to lo in NETDEV_DOWN event, so this fix is only needed when
> IP_VS_IPV6 is defined.
> 
> v1->v2:
>   - apply it only when CONFIG_IP_VS_IPV6 is defined.
> 
> Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Julian Anastasov <ja@ssi.bg>

> ---
>  net/netfilter/ipvs/ip_vs_ctl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 83395bf6..432141f 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3980,6 +3980,9 @@ static void __net_exit ip_vs_control_net_cleanup_sysctl(struct netns_ipvs *ipvs)
>  
>  static struct notifier_block ip_vs_dst_notifier = {
>  	.notifier_call = ip_vs_dst_event,
> +#ifdef CONFIG_IP_VS_IPV6
> +	.priority = ADDRCONF_NOTIFY_PRIORITY + 5,
> +#endif
>  };
>  
>  int __net_init ip_vs_control_net_init(struct netns_ipvs *ipvs)
> -- 
> 2.1.0

Regards

^ permalink raw reply

* Re: [PATCH] [PATCH net-next] tun: fix multiqueue rx
From: Jason Wang @ 2018-11-16  7:11 UTC (permalink / raw)
  To: Matthew Cover, davem, brouer, mst, edumazet, sd, netdev,
	matthew.cover
In-Reply-To: <20181116070015.1759-1-matthew.cover@stackpath.com>


On 2018/11/16 下午3:00, Matthew Cover wrote:
> When writing packets to a descriptor associated with a combined queue, the
> packets should end up on that queue.
>
> Before this change all packets written to any descriptor associated with a
> tap interface end up on rx-0, even when the descriptor is associated with a
> different queue.
>
> The rx traffic can be generated by either of the following.
>    1. a simple tap program which spins up multiple queues and writes packets
>       to each of the file descriptors
>    2. tx from a qemu vm with a tap multiqueue netdev
>
> The queue for rx traffic can be observed by either of the following (done
> on the hypervisor in the qemu case).
>    1. a simple netmap program which opens and reads from per-queue
>       descriptors
>    2. configuring RPS and doing per-cpu captures with rxtxcpu
>
> Alternatively, if you printk() the return value of skb_get_rx_queue() just
> before each instance of netif_receive_skb() in tun.c, you will get 65535
> for every skb.
>
> Calling skb_record_rx_queue() to set the rx queue to the queue_index fixes
> the association between descriptor and rx queue.
>
> Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
> ---
>   drivers/net/tun.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index a65779c6d72f..ce8620f3ea5e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1536,6 +1536,7 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
>   
>   	if (!rx_batched || (!more && skb_queue_empty(queue))) {
>   		local_bh_disable();
> +		skb_record_rx_queue(skb, tfile->queue_index);
>   		netif_receive_skb(skb);
>   		local_bh_enable();
>   		return;
> @@ -1555,8 +1556,11 @@ static void tun_rx_batched(struct tun_struct *tun, struct tun_file *tfile,
>   		struct sk_buff *nskb;
>   
>   		local_bh_disable();
> -		while ((nskb = __skb_dequeue(&process_queue)))
> +		while ((nskb = __skb_dequeue(&process_queue))) {
> +			skb_record_rx_queue(nskb, tfile->queue_index);
>   			netif_receive_skb(nskb);
> +		}
> +		skb_record_rx_queue(skb, tfile->queue_index);
>   		netif_receive_skb(skb);
>   		local_bh_enable();
>   	}
> @@ -2452,6 +2456,7 @@ static int tun_xdp_one(struct tun_struct *tun,
>   	    !tfile->detached)
>   		rxhash = __skb_get_hash_symmetric(skb);
>   
> +	skb_record_rx_queue(skb, tfile->queue_index);
>   	netif_receive_skb(skb);
>   
>   	stats = get_cpu_ptr(tun->pcpu_stats);


Acked-by: Jason Wang <jasowang@redhat.com>

^ permalink raw reply

* Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF
From: David Ahern @ 2018-11-16  7:37 UTC (permalink / raw)
  To: Alexis Bauvin, roopa, nicolas.dichtel; +Cc: netdev, akherbouche
In-Reply-To: <41C7AE9A-5C5B-4DA0-9C58-16716F710A62@scaleway.com>

On 11/15/18 2:05 AM, Alexis Bauvin wrote:
> Le 14 nov. 2018 à 20:58, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>
>> you are making this more specific than it needs to be ....
>>
>> On 11/14/18 1:31 AM, Alexis Bauvin wrote:
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>> index 27bd586b94b0..7477b5510a04 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -208,11 +208,23 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>>> 	return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>>> }
>>>
>>> +static int vxlan_get_l3mdev(struct net *net, int ifindex)
>>> +{
>>> +	struct net_device *dev;
>>> +
>>> +	dev = __dev_get_by_index(net, ifindex);
>>> +	while (dev && !netif_is_l3_master(dev))
>>> +		dev = netdev_master_upper_dev_get(dev);
>>> +
>>> +	return dev ? dev->ifindex : 0;
>>> +}
>>
>> l3mdev_master_ifindex_by_index should work instead of defining this for
>> vxlan.
>>
>> But I do not believe you need this function.
> 
> l3mdev_master_ifindex_by_index does not recursively climbs up the master chain.
> This means that if the l3mdev is not a direct master of the device, it will not
> be found.
> 
> E.G. Calling l3mdev_master_ifindex_by_index with the index of eth0 will
> return 0:
> 
> +------+     +-----+     +----------+
> |      |     |     |     |          |
> | eth0 +-----+ br0 +-----+ vrf-blue |
> |      |     |     |     |          |
> +------+     +-----+     +----------+
> 

eth0 is not the L3/router interface in this picture; br0 is. There
should not be a need for invoking l3mdev_master_ifindex_by_index on eth0.

What device stacking are you expecting to handle with vxlan devices?
vxlan on eth0 with vxlan devices in a VRF? vxlan devices into a bridge
with the bridge (or SVI) enslaved to a VRF?


> This is because the underlying l3mdev_master_dev_rcu function fetches the master
> (br0 in this case), checks whether it is an l3mdev (which it is not), and
> returns its index if so.
> 
> So if using l3mdev_master_dev_rcu, using eth0 as a lower device will still bind
> to no specific device, thus in the default VRF.
> 
> Maybe I should have patched l3mdev_master_dev_rcu to do a recursive resolution
> (as vxlan_get_l3mdev does), but I don’t know the impact of such a change.

no, that is definitely the wrong the approach.

^ permalink raw reply

* Re: [PATCH net-next] nxp: fix trivial comment typo
From: Vladimir Zapolskiy @ 2018-11-16  9:06 UTC (permalink / raw)
  To: Andrea Claudi, netdev, davem
In-Reply-To: <6573b0d13b6df551e57e2b2504395e7c0c369c44.1542220964.git.aclaudi@redhat.com>

Hello Andrea,

On 11/14/2018 08:47 PM, Andrea Claudi wrote:
> s/rxfliterctrl/rxfilterctrl
> 
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>

thank you for the patch, but let me ask you to change the subject line by
adding the expected prefixes 'net: lpc_eth: fix trivial comment typo'.
Also it would be nice to see a simple non-empty commit description.

^ permalink raw reply

* selftests: net: udpgro.sh hangs on DUT devices running Linux -next
From: Naresh Kamboju @ 2018-11-16  9:25 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, David S. Miller, Willem de Bruijn, Anders Roxell,
	Rafael Tinoco

Kernel selftests: net: udpgro.sh hangs / waits forever on x86_64 and
arm32 devices running Linux -next. Test getting PASS on arm64 devices.

Do you see this problem ?

Short error log:
---------------------
ip6tables v1.6.1: can't initialize ip6tables table `nat': Table does
not exist (do you need to insmod?)

Short log error with debug,
------------------------------------
+ ip netns exec ns-peer-pwnOKK ip6tables -t nat -I PREROUTING -d
2001:db8::1 -j DNAT --to-destination 2001:db8::3
ip6tables v1.6.1: can't initialize ip6tables table `nat': Table does
not exist (do you need to insmod?)
Perhaps ip6tables or your kernel needs to be upgraded.
+ pid=3880
+ ip netns exec ns-peer-pwnOKK ./udpgso_bench_rx -G -b 2001:db8::1 -n 0
+ sleep 0.1
+ ip netns exec ns-peer-pwnOKK ./udpgso_bench_rx -b 2001:db8::3 -r -n 10 -l 1452
+ ./udpgso_bench_tx -l 4 -6 -D 2001:db8::1 -M 1 -s 14520 -S 0
+ kill -INT 3880
++ jobs -p
+ wait 3880 3881
# Here it is waiting forever.

Long test log,
selftests: net: udpgro.sh
[ 1009.614607] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1009.737943] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
ipv4
[ 1010.053106] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1010.162944] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 no GRO                                  ok
 no GRO chk cmsg                         ./udpgso_bench_rx: wrong
packet number! got 7, expected 10

[ 1010.562502] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1010.689950] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
failed
[ 1011.066820] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1011.188955] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO                                     ok
[ 1011.550350] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1011.673963] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO chk cmsg                            ok
[ 1012.117313] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1012.242831] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO with custom segment size            ok
[ 1012.621189] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1012.738940] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
[ 1012.896143] kauditd_printk_skb: 22 callbacks suppressed
[ 1012.896143] audit: type=1325 audit(1542288720.207:2489): table=nat
family=2 entries=0
[ 1012.909742] audit: type=1300 audit(1542288720.207:2489):
arch=c000003e syscall=313 success=yes exit=0 a0=2 a1=418424 a2=0 a3=2
items=0 ppid=7763 pid=21917 auid=4294967295 uid=0 gid=0 euid=0 suid=0
fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295
comm=\"modprobe\" exe=\"/bin/kmod\" subj=kernel key=(null)
[ 1012.936822] audit: type=1327 audit(1542288720.207:2489):
proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D0069707461626C655F6E6174
[ 1012.948653] audit: type=1325 audit(1542288720.175:2490): table=nat
family=2 entries=0
[ 1012.956524] audit: type=1300 audit(1542288720.175:2490):
arch=c000003e syscall=55 success=yes exit=0 a0=4 a1=0 a2=40
a3=7ffeb4e82700 items=0 ppid=21878 pid=21916 auid=4294967295 uid=0
gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0
ses=4294967295 comm=\"iptables\" exe=\"/usr/sbin/xtables-multi\"
subj=kernel key=(null)
[ 1012.985334] audit: type=1327 audit(1542288720.175:2490):
proctitle=69707461626C6573002D74006E6174002D4900505245524F5554494E47002D64003139322E3136382E312E31002D6A00444E4154002D2D746F2D64657374696E6174696F6E003139322E3136382E312E33
[ 1013.005799] audit: type=1325 audit(1542288720.221:2491): table=nat
family=2 entries=5
[ 1013.013652] audit: type=1300 audit(1542288720.221:2491):
arch=c000003e syscall=54 success=yes exit=0 a0=4 a1=0 a2=40 a3=b8a870
items=0 ppid=21878 pid=21916 auid=4294967295 uid=0 gid=0 euid=0 suid=0
fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=4294967295
comm=\"iptables\" exe=\"/usr/sbin/xtables-multi\" subj=kernel
key=(null)
[ 1013.042023] audit: type=1327 audit(1542288720.221:2491):
proctitle=69707461626C6573002D74006E6174002D4900505245524F5554494E47002D64003139322E3136382E312E31002D6A00444E4154002D2D746F2D64657374696E6174696F6E003139322E3136382E312E33
 GRO with custom segment size cmsg       ok
 bad GRO lookup                          ok
[ 1013.331549] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1013.455957] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
ipv6
 no GRO                                  ./udpgso_bench_rx: wrong
packet number! got 7, expected 10

[ 1013.836277] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1013.952959] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
failed
[ 1014.338978] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1014.462967] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 no GRO chk cmsg                         ok
[ 1014.839919] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1014.965962] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO                                     ok
[ 1015.334213] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1015.457938] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO chk cmsg                            ok
[ 1015.823508] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1015.935954] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO with custom segment size            ok
[ 1016.332459] IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
[ 1016.453954] IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
 GRO with custom segment size cmsg       ok
 bad GRO lookup                          ip6tables v1.6.1: can't
initialize ip6tables table `nat': Table does not exist (do you need to
insmod?)

Best regards
Naresh Kamboju

^ permalink raw reply

* [PATCH] socket: do a generic_file_splice_read when proto_ops has no splice_read
From: kaslevs @ 2018-11-16  9:27 UTC (permalink / raw)
  To: netdev; +Cc: davem

From: Slavomir Kaslev <kaslevs@vmware.com>

splice(2) fails with -EINVAL when called reading on a socket with no splice_read
set in its proto_ops (such as vsock sockets). Switch this to fallbacks to a
generic_file_splice_read instead.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 net/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index 593826e11a53..334fcc617ef2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -853,7 +853,7 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 	struct socket *sock = file->private_data;
 
 	if (unlikely(!sock->ops->splice_read))
-		return -EINVAL;
+		return generic_file_splice_read(file, ppos, pipe, len, flags);
 
 	return sock->ops->splice_read(sock, ppos, pipe, len, flags);
 }
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH bpf] bpf: fix off-by-one error in adjust_subprog_starts
From: Dmitry Vyukov @ 2018-11-16 19:50 UTC (permalink / raw)
  To: Edward Cree
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, LKML, syzkaller-bugs
In-Reply-To: <bce0322a-6392-3fd4-a6fb-562160c26198@solarflare.com>

On Fri, Nov 16, 2018 at 4:00 AM, Edward Cree <ecree@solarflare.com> wrote:
> When patching in a new sequence for the first insn of a subprog, the start
>  of that subprog does not change (it's the first insn of the sequence), so
>  adjust_subprog_starts should check start <= off (rather than < off).
> Also added a test to test_verifier.c (it's essentially the syz reproducer).
>
> Fixes: cc8b0b92a169 ("bpf: introduce function calls (function boundaries)")
> Reported-by: syzbot+4fc427c7af994b0948be@syzkaller.appspotmail.com
> Signed-off-by: Edward Cree <ecree@solarflare.com>
>
> ---
> I'm assuming I don't need to get a Signed-off-by from syzkaller to use its
>  reproducer like this; I'm not an expert on the copyright niceties of works
>  written by bots.

My understanding is that copyright applies only if you directly reuse
the code (e.g. copy-paste and change). Copyright does not cover ideas
nor algorithms.
The test does not look like syzkaller reproducer. If you wrote the
test yourself, it should not have any issues with copyright. But I am
not a layer too.

The intention is that you can reuse it. I don't know if/what we need
to do to make it "official".

Thanks for the quick fix.


>  kernel/bpf/verifier.c                       |  2 +-
>  tools/testing/selftests/bpf/test_verifier.c | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1971ca325fb4..6dd419550aba 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5650,7 +5650,7 @@ static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len
>                 return;
>         /* NOTE: fake 'exit' subprog should be updated as well. */
>         for (i = 0; i <= env->subprog_cnt; i++) {
> -               if (env->subprog_info[i].start < off)
> +               if (env->subprog_info[i].start <= off)
>                         continue;
>                 env->subprog_info[i].start += len - 1;
>         }
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 6f61df62f690..550b7e46bf4a 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -13896,6 +13896,25 @@ static struct bpf_test tests[] = {
>                 .prog_type = BPF_PROG_TYPE_SCHED_CLS,
>                 .result = ACCEPT,
>         },
> +       {
> +               "calls: ctx read at start of subprog",
> +               .insns = {
> +                       BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 5),
> +                       BPF_JMP_REG(BPF_JSGT, BPF_REG_0, BPF_REG_0, 0),
> +                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
> +                       BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
> +                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +                       BPF_EXIT_INSN(),
> +                       BPF_LDX_MEM(BPF_B, BPF_REG_9, BPF_REG_1, 0),
> +                       BPF_MOV64_IMM(BPF_REG_0, 0),
> +                       BPF_EXIT_INSN(),
> +               },
> +               .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
> +               .errstr_unpriv = "function calls to other bpf functions are allowed for root only",
> +               .result_unpriv = REJECT,
> +               .result = ACCEPT,
> +       },
>  };
>
>  static int probe_filter_length(const struct bpf_insn *fp)
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/bce0322a-6392-3fd4-a6fb-562160c26198%40solarflare.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 net-next 06/21] net: usb: aqc111: Introduce link management
From: Igor Russkikh @ 2018-11-16  9:53 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: David S . Miller, linux-usb@vger.kernel.org,
	netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <1d0b529e-202a-ab5f-f377-7a4db4e79df7@gmail.com>


Hi Andrew, Florian,

>>>
>>> So the point is that MAC firmware is managing SERDES and system interface link.
>>
>> Linux can manage that SERDES link between the MAC and the PHY. There
>> are two ways this can go:
>>
>> 1) You use phylib. When the PHY reports link, the adjust_link callback
>> in the MAC is called. The phydev structure contains information about
>> how you should configure the SERDES, SGMII, 2500Base-X, 5000Base-X. It
>> works, but it is not so nice.
>>
>> 2) phylink gives you a much nicer API to do the same. Again, the PHY
>> reports the link is up. phylink will then tell the MAC how to
>> configure its end of the SERDES. The problem with phylink is that it
>> expects a DT building. You don't have that, since this is a USB
>> device. But you also don't need a lot of the features of phylink like
>> SFPs, the i2c bus for the SFPs, GPIOs etc. So it should not be to hard
>> to make this work without device tree.
>>
>> By using core linux code, we avoid bugs in firmware which nobody can
>> fix. The Linux core code should be well tested and supported, but
>> phylink is rather new, so might still have some corner cases.
>>
>> I also cannot imaging parts of the PHY driver will not be re-usable
>> for other Aquantia PHYs. I have a board with an AQCS109 under my desk
>> waiting for me to do something with it. I really would like a better
>> PHY driver for it than the kernel currently has. Hopefully there is
>> some code reuse possibilities here.
> 
> Even if the firmware is helping, there is still value in using PHYLINK
> to report things to Linux as Andrew is saying in that you get a lot of
> things for free: auto-negotiation results, link_ksetttings_{get,set} etc.

Thanks for the detailed explanation, I agree separating phy logic would
really improve things.

But just in time we've got a new updates from HW management/system teams,
It looks like they are deprecating direct phy access firmware mode,
so the only thing we have to support in this driver is FW based interface
for the link management.

Production dongles will always have firmware fully controlling all the phy.
Thus, I think in next series we'll just cut off all the direct phy
access code.

I agree with Andrew we eventually need a better generic Aquantia PHYs support
in linux, hope to look into this matter soon.

Regards,
  Igor

^ permalink raw reply

* Compliment of the day to you Dear Friend.
From: Mrs Amina.Kadi @ 2018-11-16  9:58 UTC (permalink / raw)

In-Reply-To: <1208738430.1553743.1542362323649.ref@mail.yahoo.com>



Compliment of the day to you Dear Friend.

Dear Friend.
 
  I am Mrs. Amina Kadi. am sending this brief letter to solicit your
partnership to transfer $5.5 million US Dollars. I shall send you
more information and procedures when I receive positive response from
you.

Mrs. Amina Kadi

^ permalink raw reply

* RE: [PATCH net-next v4 0/6] net/ncsi: Allow enabling multiple packages & channels
From: Justin.Lee1 @ 2018-11-16 20:17 UTC (permalink / raw)
  To: sam, netdev; +Cc: davem, linux-kernel, openbmc
In-Reply-To: <20181116045159.4980-1-sam@mendozajonas.com>

Hi Samuel,

I have reviewed/tested it. It works well. Thanks for supporting these features!

Justin


Reviewed-by: Justin Lee <justin.lee1@dell.com>
Tested-by: Justin Lee <justin.lee1@dell.com>


> This series extends the NCSI driver to configure multiple packages and/or channels simultaneously. Since the RFC series this includes a few extra changes to fix areas in the driver that either made this harder or were roadblocks due to deviations from the NCSI specification.
> 
> Patches 1 & 2 fix two issues where the driver made assumptions about the capabilities of the NCSI topology.
> Patches 3 & 4 change some internal semantics slightly to make multi-mode easier.
> Patch 5 introduces a cleaner way of reconfiguring the NCSI configuration and keeping track of channel states.
> Patch 6 implements the main multi-package/multi-channel configuration, configured via the Netlink interface.
> 
> Readers who have an interesting NCSI setup - especially multi-package with HWA - please test! I think I've covered all permutations but I don't have infinite hardware to test on.
> 
> Changes in v2:
> - Updated use of the channel lock in ncsi_reset_dev(), making the channel invisible and leaving the monitor check to ncsi_stop_channel_monitor().
> - Fixed ncsi_channel_is_tx() to consider the state of channels in other packages.
> Changes in v3:
> - Fixed bisectability bug in patch 1
> - Consider channels on all packages in a few places when multi-package is enabled.
> - Avoid doubling up reset operations, and check the current driver state before reset to let any running operations complete.
> - Reorganise the LSC handler slightly to avoid enabling Tx twice.
> Changes in v4:
> - Fix failover in the single-channel case
> - Better handle ncsi_reset_dev() entry during a current config/suspend operation.
> 
> Samuel Mendoza-Jonas (6):
>   net/ncsi: Don't enable all channels when HWA available
>   net/ncsi: Probe single packages to avoid conflict
>   net/ncsi: Don't deselect package in suspend if active
>   net/ncsi: Don't mark configured channels inactive
>   net/ncsi: Reset channel state in ncsi_start_dev()
>   net/ncsi: Configure multi-package, multi-channel modes with failover
> 
>  include/uapi/linux/ncsi.h |  15 ++
>  net/ncsi/internal.h       |  19 +-
>  net/ncsi/ncsi-aen.c       |  75 ++++--
>  net/ncsi/ncsi-manage.c    | 522 +++++++++++++++++++++++++++-----------
>  net/ncsi/ncsi-netlink.c   | 233 ++++++++++++++---
>  net/ncsi/ncsi-rsp.c       |   2 +-
>  6 files changed, 660 insertions(+), 206 deletions(-)
> 
> --
> 2.19.1

^ permalink raw reply

* Re: [PATCH net-next v2] geneve: Use empty braces for addr6 initializer
From: David Miller @ 2018-11-16 20:35 UTC (permalink / raw)
  To: sbrivio; +Cc: natechancellor, joe, sd, netdev, linux-kernel
In-Reply-To: <20181116150432.2408a075@redhat.com>

From: Stefano Brivio <sbrivio@redhat.com>
Date: Fri, 16 Nov 2018 15:04:32 +0100

> Joe, suggestions?

Let's just memset() this thing already instead of fighting the
compiler(s).

^ permalink raw reply

* Re: [PATCH 00/10] add flow_rule infrastructure
From: Or Gerlitz @ 2018-11-16 10:29 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Linux Netdev List, David Miller, thomas.lendacky,
	Florian Fainelli, Ariel Elior, Michael Chan, santosh,
	madalin.bucur, Zhuangyuzeng (Yisen), Salil Mehta, Jeff Kirsher,
	Tariq Toukan, Saeed Mahameed, Jiri Pirko, Ido Schimmel,
	Jakub Kicinski, peppe.cavallaro, grygorii.strashko, Andrew Lunn,
	Vivien Didelot, alexandre.torgue, joabreu
In-Reply-To: <20181116014139.25438-1-pablo@netfilter.org>

On Fri, Nov 16, 2018 at 3:43 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> This patchset introduces a kernel intermediate representation (IR) to
> express ACL hardware offloads, this is heavily based on the existing
> flow dissector infrastructure and the TC actions. This IR can be used by
> different frontend ACL interfaces such as ethtool_rxnfc and tc to

any reason to keep aRFS out?

> represent ACL hardware offloads. Main goal is to simplify the
> development of ACL hardware offloads for the existing frontend
> interfaces, the idea is that driver developers do not need to add one
> specific parser for each ACL frontend, instead each frontend can just
> generate this flow_rule IR and pass it to drivers to populate the
> hardware IR.

yeah, but we are adding one more chain (IR), today we have

kernel frontend U/API X --> kernel parser Y --> driver --> HW API
kernel frontend U/API Z --> kernel parser W --> driver --> HW API

and we move to

kernel frontend U/API X --> kernel parser Y --> IR --> driver --> HW API
kernel frontend U/API Z --> kernel parser W --> IR --> driver --> HW API

So instead of

Y --> HW
W --> HW

we have IR --> HW while loosing the TC semantics and spirit in the drivers.

We could have normalize all the U/APIs to use the flow dissectors and
the TC actions and then have the drivers to deal with TC only.

IMHO flow dissectors and TC actions are the right approach to deal with ACLs
HW offloading. They properly reflect how ACLs work in modern HW pipelines.

>
>                 .   ethtool_rxnfc   tc
>                |       (ioctl)    (netlink)
>                |          |         |     translate native
>       Frontend |          |         |  interface representation
>                |          |         |      to flow_rule IR
>                |          |         |
>                 .         \/        \/
>                 .         flow_rule IR
>                |                |
>        Drivers |                | parsing of flow_rule IR
>                |                |  to populate hardware IR
>                |               \/
>                 .      hardware IR (driver)
>
> For design and implementation details, please have a look at:
>
>         https://lwn.net/Articles/766695/

I will further look next week, but as this is not marked as RFC (and
not with net-next
on the title), can we consider this still a discussion and not final/review?

> As an example, with this patchset, it should be possible to simplify the
> existing net/qede driver which already has two parsers to populate the
> hardware IR, one for ethtool_rxnfc interface and another for tc.

I think it would be fair to ask for one such driver porting to see the
impact/benefit.

Or.

^ permalink raw reply

* Re: [RFC v1 2/3] vxlan: add support for underlay in non-default VRF
From: Alexis Bauvin @ 2018-11-16 10:41 UTC (permalink / raw)
  To: David Ahern, roopa, nicolas.dichtel; +Cc: netdev, akherbouche, Alexis Bauvin
In-Reply-To: <334599aa-c4c9-d2ef-c498-0b14230f7115@cumulusnetworks.com>

Le 16 nov. 2018 à 08:37, David Ahern <dsa@cumulusnetworks.com> a écrit :
> On 11/15/18 2:05 AM, Alexis Bauvin wrote:
>> Le 14 nov. 2018 à 20:58, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>> 
>>> you are making this more specific than it needs to be ....
>>> 
>>> On 11/14/18 1:31 AM, Alexis Bauvin wrote:
>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>> index 27bd586b94b0..7477b5510a04 100644
>>>> --- a/drivers/net/vxlan.c
>>>> +++ b/drivers/net/vxlan.c
>>>> @@ -208,11 +208,23 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>>>> 	return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>>>> }
>>>> 
>>>> +static int vxlan_get_l3mdev(struct net *net, int ifindex)
>>>> +{
>>>> +	struct net_device *dev;
>>>> +
>>>> +	dev = __dev_get_by_index(net, ifindex);
>>>> +	while (dev && !netif_is_l3_master(dev))
>>>> +		dev = netdev_master_upper_dev_get(dev);
>>>> +
>>>> +	return dev ? dev->ifindex : 0;
>>>> +}
>>> 
>>> l3mdev_master_ifindex_by_index should work instead of defining this for
>>> vxlan.
>>> 
>>> But I do not believe you need this function.
>> 
>> l3mdev_master_ifindex_by_index does not recursively climbs up the master chain.
>> This means that if the l3mdev is not a direct master of the device, it will not
>> be found.
>> 
>> E.G. Calling l3mdev_master_ifindex_by_index with the index of eth0 will
>> return 0:
>> 
>> +------+     +-----+     +----------+
>> |      |     |     |     |          |
>> | eth0 +-----+ br0 +-----+ vrf-blue |
>> |      |     |     |     |          |
>> +------+     +-----+     +----------+
>> 
> 
> eth0 is not the L3/router interface in this picture; br0 is. There
> should not be a need for invoking l3mdev_master_ifindex_by_index on eth0.
> 
> What device stacking are you expecting to handle with vxlan devices?
> vxlan on eth0 with vxlan devices in a VRF? vxlan devices into a bridge
> with the bridge (or SVI) enslaved to a VRF?

The case I am trying to cover here is the user creating a VXLAN device with eth0
as its lower device (ip link add vxlan0 type vxlan ... dev eth0), thus ignoring
the fact that it should be br0 (the actual L3 interface). In this case, the only
information available from the module's point of view is eth0. I may be wrong,
but eth0 is indirectly "part" of vrf-blue (even if it is only L2), as packets
flowing in from it would land in vrf-blue if L3.

As for the device stacking, I am only interested in the VXLAN underlay: the
VXLAN device itself could be in a specific VRF or not, it should not influence
its underlay. 

+----------+                         +---------+
|          |                         |         |
| vrf-blue |                         | vrf-red |
|          |                         |         |
+----+-----+                         +----+----+
     |                                    |
     |                                    |
+----+-----+                         +----+----+
|          |                         |         |
| br-blue  |                         | br-red  |
|          |                         |         |
+----+-----+                         +---+-+---+
     |                                   | |
     |                             +-----+ +-----+
     |                             |             |
+----+-----+                +------+----+   +----+----+
|          |  lower device  |           |   |         |
|   eth0   | <- - - - - - - | vxlan-red |   | tap-red | (... more taps)
|          |                |           |   |         |
+----------+                +-----------+   +---------+


While I don't see any use case for having a bridged uplink when using VXLAN,
someone may and would find a different behavior depending on the lower device.
In the above example, vxlan-red's lower device should be br-blue, but a user
would expect the underlay VRF (vrf-blue) to still be taken into account if eth0
was used as the lower device.

A different approach would be to check if the lower device is a bridge. If not,
fetch a potential master bridge. Then, with this L3/router interface, we fetch
the l3mdev with l3mdev_master_ifindex_by_index (if any).

> 
>> This is because the underlying l3mdev_master_dev_rcu function fetches the master
>> (br0 in this case), checks whether it is an l3mdev (which it is not), and
>> returns its index if so.
>> 
>> So if using l3mdev_master_dev_rcu, using eth0 as a lower device will still bind
>> to no specific device, thus in the default VRF.
>> 
>> Maybe I should have patched l3mdev_master_dev_rcu to do a recursive resolution
>> (as vxlan_get_l3mdev does), but I don’t know the impact of such a change.
> 
> no, that is definitely the wrong the approach.

Ok! What is the best approach in your opinion?

^ permalink raw reply

* Re: [PATCH v2 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode
From: Grygorii Strashko @ 2018-11-16 21:00 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: David S. Miller, Kishon Vijay Abraham I, Russell King - ARM Linux,
	netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Antoine Tenart, Vivek Gautam, Maxime Ripard, Chen-Yu Tsai,
	Carlo Caione, Chunfeng Yun, Matthias Brugger, Manu Gautam
In-Reply-To: <20181116111530.iqqcchtcnmlssvdp@qschulz>



On 11/16/18 5:15 AM, Quentin Schulz wrote:
> Hi Grygorii,
> 
> On Fri, Nov 09, 2018 at 05:47:53PM -0600, Grygorii Strashko wrote:
>> Convert ocelot-serdes PHY driver to use recently introduced
>> PHY_MODE_ETHERNET and phy_set_mode_ext().
>>
> 
> Thanks for the patch, it's annoying to have to map PHY_MODE_SGMII and
> PHY_INTERFACE_MODE_SGMII :)
> 
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   drivers/net/ethernet/mscc/ocelot.c   |  9 ++-------
>>   drivers/phy/mscc/phy-ocelot-serdes.c | 14 ++++++++++----
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
>> index 3238b9e..3edb608 100644
>> --- a/drivers/net/ethernet/mscc/ocelot.c
>> +++ b/drivers/net/ethernet/mscc/ocelot.c
>> @@ -472,7 +472,6 @@ static int ocelot_port_open(struct net_device *dev)
>>   {
>>   	struct ocelot_port *port = netdev_priv(dev);
>>   	struct ocelot *ocelot = port->ocelot;
>> -	enum phy_mode phy_mode;
>>   	int err;
>>   
>>   	/* Enable receiving frames on the port, and activate auto-learning of
>> @@ -484,12 +483,8 @@ static int ocelot_port_open(struct net_device *dev)
>>   			 ANA_PORT_PORT_CFG, port->chip_port);
>>   
>>   	if (port->serdes) {
>> -		if (port->phy_mode == PHY_INTERFACE_MODE_SGMII)
>> -			phy_mode = PHY_MODE_SGMII;
>> -		else
>> -			phy_mode = PHY_MODE_QSGMII;
>> -
>> -		err = phy_set_mode(port->serdes, phy_mode);
>> +		err = phy_set_mode_ext(port->serdes, PHY_MODE_ETHERNET,
>> +				       port->phy_mode);
>>   		if (err) {
>>   			netdev_err(dev, "Could not set mode of SerDes\n");
>>   			return err;
>> diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
>> index c61a9890..f525a21 100644
>> --- a/drivers/phy/mscc/phy-ocelot-serdes.c
>> +++ b/drivers/phy/mscc/phy-ocelot-serdes.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>>   #include <linux/of_platform.h>
>> +#include <linux/phy.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/regmap.h>
>> @@ -116,8 +117,10 @@ struct serdes_mux {
>>   	.mux = _mux,						\
>>   }
>>   
>> -#define SERDES_MUX_SGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_SGMII, m, c)
>> -#define SERDES_MUX_QSGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_QSGMII, m, c)
>> +#define SERDES_MUX_SGMII(i, p, m, c) \
>> +	SERDES_MUX(i, p, PHY_INTERFACE_MODE_SGMII, m, c)
>> +#define SERDES_MUX_QSGMII(i, p, m, c) \
>> +	SERDES_MUX(i, p, PHY_INTERFACE_MODE_QSGMII, m, c)
>>   
>>   static const struct serdes_mux ocelot_serdes_muxes[] = {
>>   	SERDES_MUX_SGMII(SERDES1G(0), 0, 0, 0),
>> @@ -164,12 +167,15 @@ static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>>   	unsigned int i;
>>   	int ret;
>>   
>> +	if (mode != PHY_MODE_ETHERNET)
>> +		return -EINVAL;
>> +
> 
> This works for now because we only support Ethernet muxes for now but
> this IP also supports PHY_MODE_PCIE.
> 
> It seems weird to me that the day we'll add support for PCIE muxing we
> will have in ocelot_serdes_muxes[i].mode either a PHY_INTERFACE_MODE_*
> or a PHY_MODE_PCIE. This is not an issue for now since you do the mode
> != PHY_MODE_ETHERNET just above but once we get rid of this, we only
> test for submode != ocelot_serdes_muxes[i].mode and both
> PHY_INTERFACE_MODE_1000BASEX and PHY_MODE_PCIE will have the same index
> thus might be confused.
> 
> Should we add a submode to the SERDES_MUX macro, move PHY_MODE_*MII to
> this submode and have PHY_MODE_ETHERNET/PCIE in the mode field?

Yeh. You are right. I'll update it this way.
I have no hw, so hope you will be able to help with review/testing.


-- 
regards,
-grygorii

^ permalink raw reply

* Re: [PATCH net-next 2/2] net/sched: act_police: don't use spinlock in the data path
From: Davide Caratti @ 2018-11-16 11:28 UTC (permalink / raw)
  To: Eric Dumazet, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	David S. Miller
  Cc: netdev
In-Reply-To: <6ef73ea1-2f0e-b1c5-3a0a-1742db67b59a@gmail.com>

On Thu, 2018-11-15 at 05:53 -0800, Eric Dumazet wrote:
> 
> On 11/15/2018 03:43 AM, Davide Caratti wrote:
> > On Wed, 2018-11-14 at 22:46 -0800, Eric Dumazet wrote:
> > > On 09/13/2018 10:29 AM, Davide Caratti wrote:
> > > > use RCU instead of spinlocks, to protect concurrent read/write on
> > > > act_police configuration. This reduces the effects of contention in the
> > > > data path, in case multiple readers are present.
> > > > 
> > > > Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> > > > ---
> > > >  net/sched/act_police.c | 156 ++++++++++++++++++++++++-----------------
> > > >  1 file changed, 92 insertions(+), 64 deletions(-)
> > > > 
> > > 
> > > I must be missing something obvious with this patch.
> > 
> > hello Eric,
> > 
> > On the opposite, I missed something obvious when I wrote that patch: there
> > is a race condition on tcfp_toks, tcfp_ptoks and tcfp_t_c: thank you for
> > noticing it.
> > 
> > These variables still need to be protected with a spinlock. I will do a
> > patch and evaluate if 'act_police' is still faster than a version where   
> > 2d550dbad83c ("net/sched: .... ") is reverted, and share results in the
> > next hours.
> > 
> > Ok?
> > 
> 
> SGTM, thanks.

hello,
I just finished the comparison of act_police, in the following cases:

a) revert the RCU-ification (i.e. commit 2d550dbad83c ("net/sched:
act_police: don't use spinlock in the data path"), and leave per-cpu
counters used by the rate estimator

b) keep RCU-ified configuration parameters, and protect read/update of
tcfp_toks, tcfp_ptoks and tcfp_t with a spinlock (code at the bottom  of
this message).

## Test setup:

$DEV is a 'dummy' with clsact qdisc; the following two commands,

# test police with 'rate'
$TC filter add dev $DEV egress matchall \
 action police rate 2gbit burst 100k conform-exceed pass/pass index 100

# test police with 'avrate'
$TC filter add dev prova egress estimator 1s 8s matchall \
        action police avrate 2gbit conform-exceed pass/pass index 100

are tested with the following loop:

for c in 1 2 4 8 16; do
./pktgen/pktgen_bench_xmit_mode_queue_xmit.sh -v -s 64  -t $c -n 5000000 -i $DEV
done


## Test results:

using  rate  | reverted       | patched
        $c   | act_police (a) | act_police (b)
-------------+----------------+---------------
         1   |       3364442  |      3345580  
         2   |       2703030  |      2721919  
         4   |       1130146  |      1253555
         8   |        664238  |       658777
        16   |        154026  |       155259


using avrate | reverted       | patched
        $c   | act_police (a) | act_police (b)
-------------+----------------+---------------
         1   |       3621796  |      3658472 
         2   |       3075589  |      3548135  
         4   |       2313314  |      3343717
         8   |        768458  |      3260480
        16   |        177776  |      3254128


so, 'avrate' still gets a significant improvement because the 'conform/exceed'
decision doesn't need the spinlock in this case. The estimation is probably
less accurate, because it use per-CPU variables: if this is not acceptable,
then we need to revert also 93be42f9173b ("net/sched: act_police: use per-cpu
counters").


## patch code:

-- >8 --
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 052855d..42db852 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -27,10 +27,7 @@ struct tcf_police_params {
 	u32			tcfp_ewma_rate;
 	s64			tcfp_burst;
 	u32			tcfp_mtu;
-	s64			tcfp_toks;
-	s64			tcfp_ptoks;
 	s64			tcfp_mtu_ptoks;
-	s64			tcfp_t_c;
 	struct psched_ratecfg	rate;
 	bool			rate_present;
 	struct psched_ratecfg	peak;
@@ -40,6 +37,9 @@ struct tcf_police_params {
 
 struct tcf_police {
 	struct tc_action	common;
+	s64			tcfp_toks;
+	s64			tcfp_ptoks;
+	s64			tcfp_t_c;
 	struct tcf_police_params __rcu *params;
 };
 
@@ -186,12 +186,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	}
 
 	new->tcfp_burst = PSCHED_TICKS2NS(parm->burst);
-	new->tcfp_toks = new->tcfp_burst;
-	if (new->peak_present) {
-		new->tcfp_mtu_ptoks = (s64)psched_l2t_ns(&new->peak,
-							 new->tcfp_mtu);
-		new->tcfp_ptoks = new->tcfp_mtu_ptoks;
-	}
 
 	if (tb[TCA_POLICE_AVRATE])
 		new->tcfp_ewma_rate = nla_get_u32(tb[TCA_POLICE_AVRATE]);
@@ -207,7 +201,14 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	}
 
 	spin_lock_bh(&police->tcf_lock);
-	new->tcfp_t_c = ktime_get_ns();
+	police->tcfp_t_c = ktime_get_ns();
+	police->tcfp_toks = new->tcfp_burst;
+	if (new->peak_present) {
+		new->tcfp_mtu_ptoks = (s64)psched_l2t_ns(&new->peak,
+							 new->tcfp_mtu);
+		police->tcfp_ptoks = new->tcfp_mtu_ptoks;
+	}
+
 	police->tcf_action = parm->action;
 	rcu_swap_protected(police->params,
 			   new,
@@ -255,27 +256,29 @@ static int tcf_police_act(struct sk_buff *skb, const struct tc_action *a,
 			ret = p->tcfp_result;
 			goto end;
 		}
-
+		spin_lock_bh(&police->tcf_lock);
 		now = ktime_get_ns();
-		toks = min_t(s64, now - p->tcfp_t_c, p->tcfp_burst);
+		toks = min_t(s64, now - police->tcfp_t_c, p->tcfp_burst);
 		if (p->peak_present) {
-			ptoks = toks + p->tcfp_ptoks;
+			ptoks = toks + police->tcfp_ptoks;
 			if (ptoks > p->tcfp_mtu_ptoks)
 				ptoks = p->tcfp_mtu_ptoks;
 			ptoks -= (s64)psched_l2t_ns(&p->peak,
 						    qdisc_pkt_len(skb));
 		}
-		toks += p->tcfp_toks;
+		toks += police->tcfp_toks;
 		if (toks > p->tcfp_burst)
 			toks = p->tcfp_burst;
 		toks -= (s64)psched_l2t_ns(&p->rate, qdisc_pkt_len(skb));
 		if ((toks|ptoks) >= 0) {
-			p->tcfp_t_c = now;
-			p->tcfp_toks = toks;
-			p->tcfp_ptoks = ptoks;
+			police->tcfp_t_c = now;
+			police->tcfp_toks = toks;
+			police->tcfp_ptoks = ptoks;
+			spin_unlock_bh(&police->tcf_lock);
 			ret = p->tcfp_result;
 			goto inc_drops;
 		}
+		spin_unlock_bh(&police->tcf_lock);
 	}
 
 inc_overlimits:
-- >8 --

any feedback is appreciated.
thanks!
-- 
davide

^ permalink raw reply related

* [PATCH v3 0/4] bpf: allow zero-initialising hash map seed
From: Lorenz Bauer @ 2018-11-16 11:41 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, songliubraving, Lorenz Bauer
In-Reply-To: <20181001104509.24211-1-lmb@cloudflare.com>

Allow forcing the seed of a hash table to zero, for deterministic
execution during benchmarking and testing.

Changes from v2:
* Change ordering of BPF_F_ZERO_SEED in linux/bpf.h

Comments adressed from v1:
* Add comment to discourage production use to linux/bpf.h
* Require CAP_SYS_ADMIN

Lorenz Bauer (4):
  bpf: allow zero-initializing hash map seed
  bpf: move BPF_F_QUERY_EFFECTIVE after map flags
  tools: sync linux/bpf.h
  tools: add selftest for BPF_F_ZERO_SEED

 include/uapi/linux/bpf.h                |  9 ++--
 kernel/bpf/hashtab.c                    | 13 ++++-
 tools/include/uapi/linux/bpf.h          | 13 +++--
 tools/testing/selftests/bpf/test_maps.c | 68 +++++++++++++++++++++----
 4 files changed, 84 insertions(+), 19 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH v3 1/4] bpf: allow zero-initializing hash map seed
From: Lorenz Bauer @ 2018-11-16 11:41 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, songliubraving, Lorenz Bauer
In-Reply-To: <20181116114111.31177-1-lmb@cloudflare.com>

Add a new flag BPF_F_ZERO_SEED, which forces a hash map
to initialize the seed to zero. This is useful when doing
performance analysis both on individual BPF programs, as
well as the kernel's hash table implementation.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 include/uapi/linux/bpf.h |  3 +++
 kernel/bpf/hashtab.c     | 13 +++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 47d606d744cc..8c01b89a4cb4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -269,6 +269,9 @@ enum bpf_attach_type {
 /* Flag for stack_map, store build_id+offset instead of pointer */
 #define BPF_F_STACK_BUILD_ID	(1U << 5)
 
+/* Zero-initialize hash function seed. This should only be used for testing. */
+#define BPF_F_ZERO_SEED		(1U << 6)
+
 enum bpf_stack_build_id_status {
 	/* user space need an empty entry to identify end of a trace */
 	BPF_STACK_BUILD_ID_EMPTY = 0,
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 2c1790288138..4b7c76765d9d 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -23,7 +23,7 @@
 
 #define HTAB_CREATE_FLAG_MASK						\
 	(BPF_F_NO_PREALLOC | BPF_F_NO_COMMON_LRU | BPF_F_NUMA_NODE |	\
-	 BPF_F_RDONLY | BPF_F_WRONLY)
+	 BPF_F_RDONLY | BPF_F_WRONLY | BPF_F_ZERO_SEED)
 
 struct bucket {
 	struct hlist_nulls_head head;
@@ -244,6 +244,7 @@ static int htab_map_alloc_check(union bpf_attr *attr)
 	 */
 	bool percpu_lru = (attr->map_flags & BPF_F_NO_COMMON_LRU);
 	bool prealloc = !(attr->map_flags & BPF_F_NO_PREALLOC);
+	bool zero_seed = (attr->map_flags & BPF_F_ZERO_SEED);
 	int numa_node = bpf_map_attr_numa_node(attr);
 
 	BUILD_BUG_ON(offsetof(struct htab_elem, htab) !=
@@ -257,6 +258,10 @@ static int htab_map_alloc_check(union bpf_attr *attr)
 		 */
 		return -EPERM;
 
+	if (zero_seed && !capable(CAP_SYS_ADMIN))
+		/* Guard against local DoS, and discourage production use. */
+		return -EPERM;
+
 	if (attr->map_flags & ~HTAB_CREATE_FLAG_MASK)
 		/* reserved bits should not be used */
 		return -EINVAL;
@@ -373,7 +378,11 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
 	if (!htab->buckets)
 		goto free_htab;
 
-	htab->hashrnd = get_random_int();
+	if (htab->map.map_flags & BPF_F_ZERO_SEED)
+		htab->hashrnd = 0;
+	else
+		htab->hashrnd = get_random_int();
+
 	for (i = 0; i < htab->n_buckets; i++) {
 		INIT_HLIST_NULLS_HEAD(&htab->buckets[i].head, i);
 		raw_spin_lock_init(&htab->buckets[i].lock);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 2/4] bpf: move BPF_F_QUERY_EFFECTIVE after map flags
From: Lorenz Bauer @ 2018-11-16 11:41 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, songliubraving, Lorenz Bauer
In-Reply-To: <20181116114111.31177-1-lmb@cloudflare.com>

BPF_F_QUERY_EFFECTIVE is in the middle of the flags valid
for BPF_MAP_CREATE. Move it to its own section to reduce confusion.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 include/uapi/linux/bpf.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8c01b89a4cb4..05d95290b848 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -257,9 +257,6 @@ enum bpf_attach_type {
 /* Specify numa node during map creation */
 #define BPF_F_NUMA_NODE		(1U << 2)
 
-/* flags for BPF_PROG_QUERY */
-#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
-
 #define BPF_OBJ_NAME_LEN 16U
 
 /* Flags for accessing BPF object */
@@ -272,6 +269,9 @@ enum bpf_attach_type {
 /* Zero-initialize hash function seed. This should only be used for testing. */
 #define BPF_F_ZERO_SEED		(1U << 6)
 
+/* flags for BPF_PROG_QUERY */
+#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
+
 enum bpf_stack_build_id_status {
 	/* user space need an empty entry to identify end of a trace */
 	BPF_STACK_BUILD_ID_EMPTY = 0,
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 3/4] tools: sync linux/bpf.h
From: Lorenz Bauer @ 2018-11-16 11:41 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, songliubraving, Lorenz Bauer
In-Reply-To: <20181116114111.31177-1-lmb@cloudflare.com>

Synchronize changes to linux/bpf.h from
* "bpf: allow zero-initializing hash map seed"
* "bpf: move BPF_F_QUERY_EFFECTIVE after map flags"

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/include/uapi/linux/bpf.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 852dc17ab47a..05d95290b848 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -257,9 +257,6 @@ enum bpf_attach_type {
 /* Specify numa node during map creation */
 #define BPF_F_NUMA_NODE		(1U << 2)
 
-/* flags for BPF_PROG_QUERY */
-#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
-
 #define BPF_OBJ_NAME_LEN 16U
 
 /* Flags for accessing BPF object */
@@ -269,6 +266,12 @@ enum bpf_attach_type {
 /* Flag for stack_map, store build_id+offset instead of pointer */
 #define BPF_F_STACK_BUILD_ID	(1U << 5)
 
+/* Zero-initialize hash function seed. This should only be used for testing. */
+#define BPF_F_ZERO_SEED		(1U << 6)
+
+/* flags for BPF_PROG_QUERY */
+#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
+
 enum bpf_stack_build_id_status {
 	/* user space need an empty entry to identify end of a trace */
 	BPF_STACK_BUILD_ID_EMPTY = 0,
@@ -2201,6 +2204,8 @@ union bpf_attr {
  *		**CONFIG_NET** configuration option.
  *	Return
  *		Pointer to *struct bpf_sock*, or NULL in case of failure.
+ *		For sockets with reuseport option, *struct bpf_sock*
+ *		return is from reuse->socks[] using hash of the packet.
  *
  * struct bpf_sock *bpf_sk_lookup_udp(void *ctx, struct bpf_sock_tuple *tuple, u32 tuple_size, u32 netns, u64 flags)
  *	Description
@@ -2233,6 +2238,8 @@ union bpf_attr {
  *		**CONFIG_NET** configuration option.
  *	Return
  *		Pointer to *struct bpf_sock*, or NULL in case of failure.
+ *		For sockets with reuseport option, *struct bpf_sock*
+ *		return is from reuse->socks[] using hash of the packet.
  *
  * int bpf_sk_release(struct bpf_sock *sk)
  *	Description
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 4/4] tools: add selftest for BPF_F_ZERO_SEED
From: Lorenz Bauer @ 2018-11-16 11:41 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, songliubraving, Lorenz Bauer
In-Reply-To: <20181116114111.31177-1-lmb@cloudflare.com>

Check that iterating two separate hash maps produces the same
order of keys if BPF_F_ZERO_SEED is used.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/testing/selftests/bpf/test_maps.c | 68 +++++++++++++++++++++----
 1 file changed, 57 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 4db2116e52be..9f0a5b16a246 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -258,23 +258,35 @@ static void test_hashmap_percpu(int task, void *data)
 	close(fd);
 }
 
+static int helper_fill_hashmap(int max_entries)
+{
+	int i, fd, ret;
+	long long key, value;
+
+	fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
+			    max_entries, map_flags);
+	CHECK(fd < 0,
+	      "failed to create hashmap",
+	      "err: %s, flags: 0x%x\n", strerror(errno), map_flags);
+
+	for (i = 0; i < max_entries; i++) {
+		key = i; value = key;
+		ret = bpf_map_update_elem(fd, &key, &value, BPF_NOEXIST);
+		CHECK(ret != 0,
+		      "can't update hashmap",
+		      "err: %s\n", strerror(ret));
+	}
+
+	return fd;
+}
+
 static void test_hashmap_walk(int task, void *data)
 {
 	int fd, i, max_entries = 1000;
 	long long key, value, next_key;
 	bool next_key_valid = true;
 
-	fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
-			    max_entries, map_flags);
-	if (fd < 0) {
-		printf("Failed to create hashmap '%s'!\n", strerror(errno));
-		exit(1);
-	}
-
-	for (i = 0; i < max_entries; i++) {
-		key = i; value = key;
-		assert(bpf_map_update_elem(fd, &key, &value, BPF_NOEXIST) == 0);
-	}
+	fd = helper_fill_hashmap(max_entries);
 
 	for (i = 0; bpf_map_get_next_key(fd, !i ? NULL : &key,
 					 &next_key) == 0; i++) {
@@ -306,6 +318,39 @@ static void test_hashmap_walk(int task, void *data)
 	close(fd);
 }
 
+static void test_hashmap_zero_seed(void)
+{
+	int i, first, second, old_flags;
+	long long key, next_first, next_second;
+
+	old_flags = map_flags;
+	map_flags |= BPF_F_ZERO_SEED;
+
+	first = helper_fill_hashmap(3);
+	second = helper_fill_hashmap(3);
+
+	for (i = 0; ; i++) {
+		void *key_ptr = !i ? NULL : &key;
+
+		if (bpf_map_get_next_key(first, key_ptr, &next_first) != 0)
+			break;
+
+		CHECK(bpf_map_get_next_key(second, key_ptr, &next_second) != 0,
+		      "next_key for second map must succeed",
+		      "key_ptr: %p", key_ptr);
+		CHECK(next_first != next_second,
+		      "keys must match",
+		      "i: %d first: %lld second: %lld\n", i,
+		      next_first, next_second);
+
+		key = next_first;
+	}
+
+	map_flags = old_flags;
+	close(first);
+	close(second);
+}
+
 static void test_arraymap(int task, void *data)
 {
 	int key, next_key, fd;
@@ -1534,6 +1579,7 @@ static void run_all_tests(void)
 	test_hashmap(0, NULL);
 	test_hashmap_percpu(0, NULL);
 	test_hashmap_walk(0, NULL);
+	test_hashmap_zero_seed();
 
 	test_arraymap(0, NULL);
 	test_arraymap_percpu(0, NULL);
-- 
2.17.1

^ 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