Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 00/11] AF_XDP zero-copy support for i40e
From: William Tu @ 2018-08-30  0:10 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Björn Töpel, Karlsson, Magnus, magnus.karlsson,
	Alexander Duyck, Alexander Duyck, Alexei Starovoitov,
	Jesper Dangaard Brouer, Linux Kernel Network Developers,
	Brandeburg, Jesse, Anjali Singhai Jain, peter.waskiewicz.jr,
	Björn Töpel, michael.lundkvist, Willem de Bruijn,
	John Fastabend, Jakub Kicinski, neerav.parikh, mykyta.iziumtsev
In-Reply-To: <1c26d5c6-cd37-b022-fe34-1ca48ada598f@iogearbox.net>

> Thanks for working on this, LGTM! Are you also planning to get ixgbe
> out after that?
>

I currently don't have i40e nic to test, so
I'm also looking forward to the ixgbe patch!

Thank you
William

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: net: cpsw: Document cpsw-phy-sel usage but prefer phandle
From: Grygorii Strashko @ 2018-08-30  0:08 UTC (permalink / raw)
  To: Tony Lindgren, David Miller
  Cc: netdev, linux-omap, devicetree, Andrew Lunn, Ivan Khoronzhuk,
	Mark Rutland, Murali Karicheri, Rob Herring
In-Reply-To: <20180829150024.43210-1-tony@atomide.com>

Hi Tony,

On 08/29/2018 10:00 AM, Tony Lindgren wrote:
> The current cpsw usage for cpsw-phy-sel is undocumented but is used for
> all the boards using cpsw. And cpsw-phy-sel is not really a child of
> the cpsw device, it lives in the system control module instead.
> 
> Let's document the existing usage, and improve it a bit where we prefer
> to use a phandle instead of a child device for it. That way we can
> properly describe the hardware in dts files for things like genpd.

I'm ok with this series, but I really don't like cpsw-phy-sel in general.

It was introduced long time back and now I'm thinking about possibility to replace it with
one of current generic interfaces - for example mux-controller. 
Each port will control up to 3 muxes (port mode, idmode and rmii_ext_clk) and  
transform phy-mode => mux states.
What do you think?

Another option is to use phy, but it'd be complicated.

> 
> Cc: devicetree@vger.kernel.org
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Murali Karicheri <m-karicheri2@ti.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   Documentation/devicetree/bindings/net/cpsw.txt | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -19,6 +19,10 @@ Required properties:
>   - slaves		: Specifies number for slaves
>   - active_slave		: Specifies the slave to use for time stamping,
>   			  ethtool and SIOCGMIIPHY
> +- cpsw-phy-sel		: Specifies the phandle to the CPSW phy mode selection
> +			  device. See also cpsw-phy-sel.txt for it's binding.
> +			  Note that in legacy cases cpsw-phy-sel may be
> +			  a child device instead of a phandle.
>   
>   Optional properties:
>   - ti,hwmods		: Must be "cpgmac0"
> @@ -75,6 +79,7 @@ Examples:
>   		cpts_clock_mult = <0x80000000>;
>   		cpts_clock_shift = <29>;
>   		syscon = <&cm>;
> +		cpsw-phy-sel = <&phy_sel>;
>   		cpsw_emac0: slave@0 {
>   			phy_id = <&davinci_mdio>, <0>;
>   			phy-mode = "rgmii-txid";
> @@ -103,6 +108,7 @@ Examples:
>   		cpts_clock_mult = <0x80000000>;
>   		cpts_clock_shift = <29>;
>   		syscon = <&cm>;
> +		cpsw-phy-sel = <&phy_sel>;
>   		cpsw_emac0: slave@0 {
>   			phy_id = <&davinci_mdio>, <0>;
>   			phy-mode = "rgmii-txid";
> 

-- 
regards,
-grygorii

^ permalink raw reply

* [PATCH net] ebpf: fix bpf_msg_pull_data
From: Tushar Dave @ 2018-08-30  0:07 UTC (permalink / raw)
  To: john.fastabend, ast, daniel, davem, netdev; +Cc: sowmini.varadhan

While doing some preliminary testing it is found that bpf helper
bpf_msg_pull_data does not calculate the data and data_end offset
correctly. Fix it!

Fixes: 015632bb30da ("bpf: sk_msg program helper bpf_sk_msg_pull_data")
Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
Acked-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/core/filter.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index c25eb36..3eeb3d6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2285,7 +2285,7 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 BPF_CALL_4(bpf_msg_pull_data,
 	   struct sk_msg_buff *, msg, u32, start, u32, end, u64, flags)
 {
-	unsigned int len = 0, offset = 0, copy = 0;
+	unsigned int len = 0, offset = 0, copy = 0, off = 0;
 	struct scatterlist *sg = msg->sg_data;
 	int first_sg, last_sg, i, shift;
 	unsigned char *p, *to, *from;
@@ -2299,22 +2299,30 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 	i = msg->sg_start;
 	do {
 		len = sg[i].length;
-		offset += len;
 		if (start < offset + len)
 			break;
+		offset += len;
 		i++;
 		if (i == MAX_SKB_FRAGS)
 			i = 0;
-	} while (i != msg->sg_end);
+	} while (i <= msg->sg_end);
 
+	/* return error if start is out of range */
 	if (unlikely(start >= offset + len))
 		return -EINVAL;
 
-	if (!msg->sg_copy[i] && bytes <= len)
-		goto out;
+	/* return error if i is last entry in sglist and end is out of range */
+	if (msg->sg_copy[i] && end > offset + len)
+		return -EINVAL;
 
 	first_sg = i;
 
+	/* if i is not last entry in sg list and end (i.e start + bytes) is
+	 * within this sg[i] then goto out and calculate data and data_end
+	 */
+	if (!msg->sg_copy[i] && end <= offset + len)
+		goto out;
+
 	/* At this point we need to linearize multiple scatterlist
 	 * elements or a single shared page. Either way we need to
 	 * copy into a linear buffer exclusively owned by BPF. Then
@@ -2330,9 +2338,14 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 		i++;
 		if (i == MAX_SKB_FRAGS)
 			i = 0;
-		if (bytes < copy)
+		if (end < copy)
 			break;
-	} while (i != msg->sg_end);
+	} while (i <= msg->sg_end);
+
+	/* return error if i is last entry in sglist and end is out of range */
+	if (i > msg->sg_end && end > offset + copy)
+		return -EINVAL;
+
 	last_sg = i;
 
 	if (unlikely(copy < end - start))
@@ -2342,23 +2355,22 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 	if (unlikely(!page))
 		return -ENOMEM;
 	p = page_address(page);
-	offset = 0;
 
 	i = first_sg;
 	do {
 		from = sg_virt(&sg[i]);
 		len = sg[i].length;
-		to = p + offset;
+		to = p + off;
 
 		memcpy(to, from, len);
-		offset += len;
+		off += len;
 		sg[i].length = 0;
 		put_page(sg_page(&sg[i]));
 
 		i++;
 		if (i == MAX_SKB_FRAGS)
 			i = 0;
-	} while (i != last_sg);
+	} while (i < last_sg);
 
 	sg[first_sg].length = copy;
 	sg_set_page(&sg[first_sg], page, copy, 0);
@@ -2380,7 +2392,7 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 		else
 			move_from = i + shift;
 
-		if (move_from == msg->sg_end)
+		if (move_from > msg->sg_end)
 			break;
 
 		sg[i] = sg[move_from];
@@ -2396,7 +2408,7 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
 	if (msg->sg_end < 0)
 		msg->sg_end += MAX_SKB_FRAGS;
 out:
-	msg->data = sg_virt(&sg[i]) + start - offset;
+	msg->data = sg_virt(&sg[first_sg]) + start - offset;
 	msg->data_end = msg->data + bytes;
 
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH 0/5] net: mvneta: some bug fix and trivial improvement
From: Jisheng Zhang @ 2018-08-30  3:53 UTC (permalink / raw)
  To: thomas.petazzoni, David S. Miller
  Cc: netdev, Gregory CLEMENT, linux-kernel, linux-arm-kernel
In-Reply-To: <20180829165131.52798cd6@xhacker.debian>

On Wed, 29 Aug 2018 16:51:31 +0800 Jisheng Zhang wrote:

> On Wed, 29 Aug 2018 16:40:24 +0800 Jisheng Zhang wrote:
> 
> > On Wed, 29 Aug 2018 16:25:57 +0800
> > Jisheng Zhang wrote:
> >   
> > > patch1 fixes rx_offset_correction set and usage. Because the
> > > rx_offset_correction is RX packet offset correction for platforms,
> > > it's not related with SW BM, instead, it's only related with the
> > > platform's NET_SKB_PAD.
> > > 
> > > patch2 fixes the wrong function to unmap rx buf    
> > 
> > I have question about the following two commits:
> > 
> > 7e47fd84b56b ("net: mvneta: Allocate page for the descriptor"), it cause
> > a waste, for normal 1500 MTU, before this patch we allocate 1920Bytes for rx
> > after this patch, we always allocate PAGE_SIZE bytes, if PAGE_SIZE=4096, we
> > waste 53% memory for each rx buf. I'm not sure whether the performance
> > improvement deserve the pay.
> > 
> > 562e2f467e71 ("net: mvneta: Improve the buffer allocation method for SWBM")
> > mentions that "With system having a small memory (around 256MB), the state
> > "cannot allocate memory to refill with new buffer" is reach pretty quickly"
> > is it due to the memory waste as said above? Anyway, by this commit, we
> > want to improve the situation on a small memory system, so should we firstly
> > revert commit 7e47fd84b56b ("net: mvneta: Allocate page for the descriptor")?

Any comments? 

Now I believe the situation is due to the memory waste introduced by 7e47fd84b56b
With linux 4.18, I tried to limit berlin platforms available memory to 256MB,
I didn't see "cannot allocate memory to refill with new buffer".

Thanks

> >   
> 
> If maintainers decide to revert the two commits: 7e47fd84b56b and 562e2f467e71
> then, patch1,2,3 are useless, we can drop them. Only patch4 and patch5 are
> still useful.
> 
> Thanks
> 
> > Any comments are welcome!
> > 
> > Thanks
> > 
> >   
> > > 
> > > patch3 removes the NETIF_F_GRO check ourself, because the net subsystem
> > > will handle it for us.
> > > 
> > > patch4 enables NETIF_F_RXCSUM by default, since the driver and HW
> > > supports the feature.
> > > 
> > > patch5 is a trivial optimization, to reduce smp_processor_id() calling
> > > in mvneta_tx_done_gbe.
> > > 
> > > Jisheng Zhang (5):
> > >   net: mvneta: fix rx_offset_correction set and usage
> > >   net: mvneta: fix the wrong function to unmap rx buf
> > >   net: mvneta: Don't check NETIF_F_GRO ourself
> > >   net: mvneta: enable NETIF_F_RXCSUM by default
> > >   net: mvneta: reduce smp_processor_id() calling in mvneta_tx_done_gbe
> > > 
> > >  drivers/net/ethernet/marvell/mvneta.c | 49 ++++++++++++---------------
> > >  1 file changed, 22 insertions(+), 27 deletions(-)
> > >     
> >   
> 

^ permalink raw reply

* [PATCH net-next] net/ipv6: Do not reset nl_net in ip6_route_info_create
From: dsahern @ 2018-08-29 23:54 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern

From: David Ahern <dsahern@gmail.com>

nl_net is set on entry to ip6_route_info_create. Only devices
within that namespace are considered so no need to reset it
before returning.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/route.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c4ea13e8360b..b5f385d2b0e9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3138,8 +3138,6 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
 	rt->fib6_nh.nh_dev = dev;
 	rt->fib6_table = table;
 
-	cfg->fc_nlinfo.nl_net = dev_net(dev);
-
 	if (idev)
 		in6_dev_put(idev);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next] net/ipv4: Add extack message that dev is required for ONLINK
From: dsahern @ 2018-08-29 23:53 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern

From: David Ahern <dsahern@gmail.com>

Make IPv4 consistent with IPv6 and return an extack message that the
ONLINK flag requires a nexthop device.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/fib_semantics.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f3c89ccf14c5..bee8db979195 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -797,8 +797,10 @@ static int fib_check_nh(struct fib_config *cfg, struct fib_nh *nh,
 				return -EINVAL;
 			}
 			dev = __dev_get_by_index(net, nh->nh_oif);
-			if (!dev)
+			if (!dev) {
+				NL_SET_ERR_MSG(extack, "Nexthop device required for onlink");
 				return -ENODEV;
+			}
 			if (!(dev->flags & IFF_UP)) {
 				NL_SET_ERR_MSG(extack,
 					       "Nexthop device is not up");
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 4/5] net: mvneta: enable NETIF_F_RXCSUM by default
From: Andrew Lunn @ 2018-08-30  3:44 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: netdev, Gregory CLEMENT, linux-kernel, thomas.petazzoni,
	David S. Miller, linux-arm-kernel
In-Reply-To: <20180830112745.61502abf@xhacker.debian>

> > I've seen network interfaces get checksum'ing wrong when used with an
> > Ethernet switch with DSA. The extra header DSA uses means the hardware
> > cannot parse the packet correctly, and so cannot find these headers.
> 
> The network interface is mvneta?

I've not tested mvneta yet. It could be, since it was designed to be
used with Marvell switches in things like WiFi boxes, it knows how to
find the IP header when there is a DSA tag.

> Do you mean after this patch, we would see errors as the following?
> 
> "bad rx status 0xabcdefgh (crc error)"

I've not tried it yet. That is why i'm trying to understand what
NETIF_F_RXCSUM actually means.

       Andrew

^ permalink raw reply

* Re: [PATCH 0/5] net: mvneta: some bug fix and trivial improvement
From: Jisheng Zhang @ 2018-08-30  3:42 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: thomas.petazzoni, David S. Miller, netdev, linux-kernel,
	Gregory CLEMENT, linux-arm-kernel
In-Reply-To: <20180829131232.GE1955@lunn.ch>

On Wed, 29 Aug 2018 15:12:32 +0200 Andrew Lunn wrote:

> Hi Jisheng
> 
> Please separate fixes from new features.
> 
> Fixes should be based on DaveM net branch, and use the subject line
> [PATCH net]...
> 
> New features should be based on DaveM net-next branch, and use the
> subject line [PATCH net-next]...

Got it. Thanks for information.

^ permalink raw reply

* Re: [PATCH 2/5] net: mvneta: fix the wrong function to unmap rx buf
From: Jisheng Zhang @ 2018-08-30  3:40 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: thomas.petazzoni, David S. Miller, netdev, linux-kernel,
	linux-arm-kernel, Andrew Lunn
In-Reply-To: <87a7p5jzp3.fsf@bootlin.com>

On Wed, 29 Aug 2018 11:21:12 +0200 Gregory CLEMENT  wrote:

> Hi Jisheng,
>  
>  On mer., août 29 2018, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
> 
> > Commit 7e47fd84b56b ("net: mvneta: Allocate page for the descriptor")
> > always allocate one page for each rx descriptor, so the rx is mapped
> > with dmap_map_page() now, but the unmap routine isn't updated at the
> > same time.
> >
> > Fix this by using dma_unmap_page() in corresponding places.
> >
> > Fixes: 7e47fd84b56b ("net: mvneta: Allocate page for the descriptor")
> > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > ---
> >  drivers/net/ethernet/marvell/mvneta.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index 0ce94f6587a5..d9206094fce3 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -1890,8 +1890,9 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
> >  		if (!data || !(rx_desc->buf_phys_addr))
> >  			continue;
> >  
> > -		dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> > -				 MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
> > +		dma_unmap_page(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> > +			       MVNETA_RX_BUF_SIZE(pp->pkt_size),
> > +			       DMA_FROM_DEVICE);
> >  		__free_page(data);
> >  	}
> >  }  
> This one can be called when the allocation is done in with HWBM in this
> case which use a dma_map_single.

oops, thanks for the catch. will fix it in v2

> 
> Gregory
> 
> 
> 
> > @@ -2008,8 +2009,8 @@ static int mvneta_rx_swbm(struct napi_struct *napi,
> >  				skb_add_rx_frag(rxq->skb, frag_num, page,
> >  						frag_offset, frag_size,
> >  						PAGE_SIZE);
> > -				dma_unmap_single(dev->dev.parent, phys_addr,
> > -						 PAGE_SIZE, DMA_FROM_DEVICE);
> > +				dma_unmap_page(dev->dev.parent, phys_addr,
> > +					       PAGE_SIZE, DMA_FROM_DEVICE);
> >  				rxq->left_size -= frag_size;
> >  			}
> >  		} else {
> > @@ -2039,9 +2040,8 @@ static int mvneta_rx_swbm(struct napi_struct *napi,
> >  						frag_offset, frag_size,
> >  						PAGE_SIZE);
> >  
> > -				dma_unmap_single(dev->dev.parent, phys_addr,
> > -						 PAGE_SIZE,
> > -						 DMA_FROM_DEVICE);
> > +				dma_unmap_page(dev->dev.parent, phys_addr,
> > +					       PAGE_SIZE, DMA_FROM_DEVICE);
> >  
> >  				rxq->left_size -= frag_size;
> >  			}
> > -- 
> > 2.18.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel  
> 

^ permalink raw reply

* Re: [PATCH 4/5] net: mvneta: enable NETIF_F_RXCSUM by default
From: Jisheng Zhang @ 2018-08-30  3:27 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Gregory CLEMENT, linux-kernel, thomas.petazzoni,
	David S. Miller, linux-arm-kernel
In-Reply-To: <20180829130836.GD1955@lunn.ch>

Hi Andrew,

On Wed, 29 Aug 2018 15:08:36 +0200 Andrew Lunn wrote:

> On Wed, Aug 29, 2018 at 04:29:32PM +0800, Jisheng Zhang wrote:
> > The code and HW supports NETIF_F_RXCSUM, so let's enable it by default.  
> 
> Hi Jisheng
> 
> I've never studied what all these different flags mean. Does
> NETIF_F_RXCSUM mean Ethernet FCS? Or does it also include IPv4, IPv6,
> UDP, TCP... checksums?

Per my understanding, it means RX checksumming. And it only supports IPv4
RX checksum, the code will

> 
> I've seen network interfaces get checksum'ing wrong when used with an
> Ethernet switch with DSA. The extra header DSA uses means the hardware
> cannot parse the packet correctly, and so cannot find these headers.

The network interface is mvneta? Do you mean after this patch, we would
see errors as the following?

"bad rx status 0xabcdefgh (crc error)"

So for DSA, we should disable RXCSUM? I'm not sure how to handle this case.
I believe other drivers(with RXCSUM enabled by deafult) also have this problem
with DSA.

Thanks

> 
> If this is just for FCS, then it is not a problem.
> 
>    Thanks
> 	Andrew
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: pull-request: bpf 2018-08-29
From: David Miller @ 2018-08-29 23:21 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev
In-Reply-To: <20180829190724.17254-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 29 Aug 2018 21:07:24 +0200

> The following pull-request contains BPF updates for your *net* tree.
> 
> The main changes are:
> 
> 1) Fix a build error in sk_reuseport_convert_ctx_access() when
>    compiling with clang which cannot resolve hweight_long() at
>    build time inside the BUILD_BUG_ON() assertion, from Stefan.
> 
> 2) Several fixes for BPF sockmap, four of them in getting the
>    bpf_msg_pull_data() helper to work, one use after free case
>    in bpf_tcp_close() and one refcount leak in bpf_tcp_recvmsg(),
>    from Daniel.
> 
> 3) Another fix for BPF sockmap where we misaccount sk_mem_uncharge()
>    in the socket redirect error case from unwinding scatterlist
>    twice, from John.
> 
> Please consider pulling these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git

Pulled, thanks Daniel.

^ permalink raw reply

* Re: [PATCH V1] add huawei ibma driver modules The driver is used for communication between in-band management agent(iBMA) and out-of-band management controller(iBMC) via pcie bus in Huawei V3 server. The driver provides character device,VNIC and black box interface for application layer.
From: Andrew Lunn @ 2018-08-30  3:17 UTC (permalink / raw)
  To: xiongsujuan
  Cc: davem, zhaochen6, aviad.krawczyk, romain.perier, bhelgaas,
	keescook, colin.king, netdev, linux-kernel
In-Reply-To: <1535591472-44997-1-git-send-email-xiongsujuan.xiongsujuan@huawei.com>

On Thu, Aug 30, 2018 at 09:11:12AM +0800, xiongsujuan wrote:

Hi xiongsujuan

Please add a real commit message. Talk about the architecture, etc.

You are also missing a Signed-off-by:

> +const struct file_operations g_bma_cdev_fops = {
> +	.owner = THIS_MODULE,
> +	.open = cdev_open,
> +	.release = cdev_release,
> +	.poll = cdev_poll,
> +	.read = cdev_read,
> +	.write = cdev_write,
> +};
> +
> +static int __init bma_cdev_init(void)
> +{
> +	int i = 0;
> +
> +	int ret = 0;
> +	int err_count = 0;
> +
> +	if (!bma_intf_check_edma_supported())
> +		return -ENXIO;
> +
> +	if (dev_num <= 0 || dev_num > CDEV_MAX_NUM)
> +		return -EINVAL;
> +
> +	memset(&g_cdev_set, 0, sizeof(struct cdev_dev_set));
> +	g_cdev_set.dev_num = dev_num;
> +
> +	for (i = 0; i < dev_num; i++) {
> +		struct cdev_dev *pDev = &g_cdev_set.dev_list[i];
> +
> +		sprintf(pDev->dev_name, "%s%d", CDEV_NAME_PREFIX, i);
> +		pDev->dev_struct.name = pDev->dev_name;
> +		pDev->dev_struct.minor = MISC_DYNAMIC_MINOR;
> +		pDev->dev_struct.fops = &g_bma_cdev_fops;
> +
> +		pDev->dev_id = CDEV_INVALID_ID;
> +
> +		ret = misc_register(&pDev->dev_struct);
> +
> +		if (ret) {
> +			CDEV_LOG(DLOG_DEBUG, "misc_register failed %d", i);
> +			err_count++;
> +			continue;
> +		}
> +
> +		pDev->dev_id = MKDEV(MISC_MAJOR, pDev->dev_struct.minor);
> +
> +		ret = bma_intf_register_type(TYPE_CDEV + i, 0, INTR_DISABLE,
> +					     &pDev->dev_data);
> +
> +		if (ret) {
> +			CDEV_LOG(DLOG_ERROR,
> +				 "cdev %d open failed ,result = %d",
> +				 i, ret);
> +		misc_deregister(&pDev->dev_struct);

I've never seen a Ethernet driver with a cdev. You need to explain
this. What is it, why does it exist, etc.

      Andrew

^ permalink raw reply

* Re: [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice
From: Andrew Lunn @ 2018-08-30  3:11 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: davem, keescook, f.fainelli, linux-kernel, netdev, alex.williams
In-Reply-To: <20180830004046.9417-3-mdf@kernel.org>

On Wed, Aug 29, 2018 at 05:40:45PM -0700, Moritz Fischer wrote:
> Add support for instantiating nixge as subdevice using
> fixed-link and platform data to configure it.
> 
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> ---
> 
> Hi,
> 
> not this patch is still in the early stages,
> and as the rest of this series goes on top of [1].
> 
> The actual platform data might still change since
> the parent device driver is still under development.

Hi Moritz

Could you tell us more about the parent device. I'm guessing PCIe.  Is
it x86 so no device tree? Are there cases where it does not have a PHY
connected? What is connected instead? SFP? A switch? Can there be
multiple PHYs on the MDIO bus?

Answering these questions will help decide how best to structure this.

	 Andrew

^ permalink raw reply

* Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes
From: Andrew Lunn @ 2018-08-30  3:04 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: davem, keescook, f.fainelli, linux-kernel, netdev, alex.williams
In-Reply-To: <20180830004046.9417-2-mdf@kernel.org>

On Wed, Aug 29, 2018 at 05:40:44PM -0700, Moritz Fischer wrote:
> Add support for fixed-link cases where no MDIO is
> actually required to run the device.
> In that case no MDIO bus is instantiated since the
> actual registers are not available in hardware.

Hi Moritz

There are a few different use cases here:

The hardware is missing MDIO - You need fixed-link.

The hardware has MDIO, but you don't have a PHY connected on it, and
use fixed link.

The hardware has MDIO, and it is used e.g. for an Ethernet switch, or
a PHY for another Ethernet interface. Plus you need fixed link.

The binding typically looks like:

&fec1 {
        phy-mode = "rmii";
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_fec1>;
        status = "okay";

        fixed-link {
                speed = <100>;
                full-duplex;
        };

        mdio1: mdio {
                #address-cells = <1>;
                #size-cells = <0>;
                status = "okay";

                switch0: switch0@0 {
                        compatible = "marvell,mv88e6085";
                        pinctrl-names = "default";
                        pinctrl-0 = <&pinctrl_switch>;
                        reg = <0>;
                        eeprom-length = <512>;
                        interrupt-parent = <&gpio3>;

It is important you have the mdio subnode, with PHYs and switches as
children. The driver currently gets this wrong, it uses
pdev->dev.of_node.

So the first patch should be to extend this behaviour. Look for a
child node called mdio. If it exists, call nixge_mdio_setup() passing
that child. Otherwise continue using pdev->dev.of_node, so you don't
break backwards compatibility.

Then a patch adding support for fixed-link. If the mdio child node
exists, you still need to register the MDIO bus. If there is no child
node, but there is a fixed-link, skip registering the mdio bus with
pdev->dev.of_node.

	Andrew

^ permalink raw reply

* Re: [PATCH net-next] net: nixge: Add support for 64-bit platforms
From: David Miller @ 2018-08-30  2:55 UTC (permalink / raw)
  To: mdf; +Cc: keescook, netdev, linux-kernel, alex.williams, f.fainelli
In-Reply-To: <20180828221631.14573-1-mdf@kernel.org>

From: Moritz Fischer <mdf@kernel.org>
Date: Tue, 28 Aug 2018 15:16:31 -0700

> Add support for 64-bit platforms to driver.
> 
> The hardware only supports 32-bit register accesses
> so the accesses need to be split up into two writes
> when setting the current and tail descriptor values.
> 
> Signed-off-by: Moritz Fischer <mdf@kernel.org>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] net: bcmgenet: use MAC link status for fixed phy
From: David Miller @ 2018-08-30  2:52 UTC (permalink / raw)
  To: opendmb; +Cc: f.fainelli, netdev, linux-kernel
In-Reply-To: <1535484795-31871-1-git-send-email-opendmb@gmail.com>

From: Doug Berger <opendmb@gmail.com>
Date: Tue, 28 Aug 2018 12:33:15 -0700

> When using the fixed PHY with GENET (e.g. MOCA) the PHY link
> status can be determined from the internal link status captured
> by the MAC. This allows the PHY state machine to use the correct
> link state with the fixed PHY even if MAC link event interrupts
> are missed when the net device is opened.
> 
> Fixes: 8d88c6e ("net: bcmgenet: enable MoCA link state change detection")
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Applied with fixed Fixes: tag SHA1-ID and queued up for -stable.

^ permalink raw reply

* [net-next 15/15] i40e: Prevent deleting MAC address from VF when set by PF
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Patryk Małek, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Patryk Małek <patryk.malek@intel.com>

To prevent VF from deleting MAC address that was assigned by the
PF we need to check for that scenario when we try to delete a MAC
address from a VF.

Signed-off-by: Patryk Małek <patryk.malek@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index f56c645588f3..3e707c7e6782 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -2569,6 +2569,16 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen)
 			ret = I40E_ERR_INVALID_MAC_ADDR;
 			goto error_param;
 		}
+
+		if (vf->pf_set_mac &&
+		    ether_addr_equal(al->list[i].addr,
+				     vf->default_lan_addr.addr)) {
+			dev_err(&pf->pdev->dev,
+				"MAC addr %pM has been set by PF, cannot delete it for VF %d, reset VF to change MAC addr\n",
+				vf->default_lan_addr.addr, vf->vf_id);
+			ret = I40E_ERR_PARAM;
+			goto error_param;
+		}
 	}
 	vsi = pf->vsi[vf->lan_vsi_idx];
 
-- 
2.17.1

^ permalink raw reply related

* [net-next 11/15] i40e: Check and correct speed values for link on open
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Jan Sokolowski, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Jan Sokolowski <jan.sokolowski@intel.com>

If our card has been put in an unstable state due to
other drivers interacting with it, speed settings
might be incorrect. If incorrect, forcefully reset them
on open to known default values.

Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 27 ++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6cb4076e8fba..c30feb27e1c0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6570,6 +6570,24 @@ static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool is_up)
 	struct i40e_hw *hw = &pf->hw;
 	i40e_status err;
 	u64 mask;
+	u8 speed;
+
+	/* Card might've been put in an unstable state by other drivers
+	 * and applications, which causes incorrect speed values being
+	 * set on startup. In order to clear speed registers, we call
+	 * get_phy_capabilities twice, once to get initial state of
+	 * available speeds, and once to get current PHY config.
+	 */
+	err = i40e_aq_get_phy_capabilities(hw, false, true, &abilities,
+					   NULL);
+	if (err) {
+		dev_err(&pf->pdev->dev,
+			"failed to get phy cap., ret =  %s last_status =  %s\n",
+			i40e_stat_str(hw, err),
+			i40e_aq_str(hw, hw->aq.asq_last_status));
+		return err;
+	}
+	speed = abilities.link_speed;
 
 	/* Get the current phy config */
 	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
@@ -6583,9 +6601,9 @@ static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool is_up)
 	}
 
 	/* If link needs to go up, but was not forced to go down,
-	 * no need for a flap
+	 * and its speed values are OK, no need for a flap
 	 */
-	if (is_up && abilities.phy_type != 0)
+	if (is_up && abilities.phy_type != 0 && abilities.link_speed != 0)
 		return I40E_SUCCESS;
 
 	/* To force link we need to set bits for all supported PHY types,
@@ -6597,7 +6615,10 @@ static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool is_up)
 	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
 	/* Copy the old settings, except of phy_type */
 	config.abilities = abilities.abilities;
-	config.link_speed = abilities.link_speed;
+	if (abilities.link_speed != 0)
+		config.link_speed = abilities.link_speed;
+	else
+		config.link_speed = speed;
 	config.eee_capability = abilities.eee_capability;
 	config.eeer = abilities.eeer_val;
 	config.low_power_ctrl = abilities.d3_lpan;
-- 
2.17.1

^ permalink raw reply related

* [net-next 08/15] virtchnl: use u8 type for a field in the virtchnl_filter struct
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem
  Cc: Harshitha Ramamurthy, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>

The virtchnl_filter struct has a field called field_flags. A previous
commit mistakenly had the type to be a __u8. What we want is for the
field to be an unsigned 8 bit value, so let's just use the existing
kernel type u8 for that.

Signed-off-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 include/linux/avf/virtchnl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index 212b3822d180..b41f7bc958ef 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -573,7 +573,7 @@ struct virtchnl_filter {
 	enum	virtchnl_flow_type flow_type;
 	enum	virtchnl_action action;
 	u32	action_meta;
-	__u8	field_flags;
+	u8	field_flags;
 };
 
 VIRTCHNL_CHECK_STRUCT_LEN(272, virtchnl_filter);
-- 
2.17.1

^ permalink raw reply related

* [net-next 14/15] i40evf: cancel workqueue sync for adminq when a VF is removed
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Lihong Yang, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Lihong Yang <lihong.yang@intel.com>

If a VF is being removed, there is no need to continue with the
workqueue sync for the adminq task, thus cancel it. Without this call,
when VFs are created and removed right away, there might be a chance for
the driver to crash with events stuck in the adminq.

Signed-off-by: Lihong Yang <lihong.yang@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index c7048cf484fb..174d1da2857b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -3910,6 +3910,8 @@ static void i40evf_remove(struct pci_dev *pdev)
 	if (adapter->watchdog_timer.function)
 		del_timer_sync(&adapter->watchdog_timer);
 
+	cancel_work_sync(&adapter->adminq_task);
+
 	i40evf_free_rss(adapter);
 
 	if (hw->aq.asq.count)
-- 
2.17.1

^ permalink raw reply related

* [net-next 10/15] i40e: report correct statistics when XDP is enabled
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem
  Cc: Björn Töpel, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

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

When XDP is enabled, the driver will report incorrect
statistics. Received frames will reported as transmitted frames.

This commits fixes the i40e implementation of ndo_get_stats64 (struct
net_device_ops), so that iproute2 will report correct statistics
(e.g. when running "ip -stats link show dev eth0") even when XDP is
enabled.

Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Fixes: 74608d17fe29 ("i40e: add support for XDP_TX action")
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 24 +++++++++++----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 62f2b5bce6bb..6cb4076e8fba 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -420,9 +420,9 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
 				  struct rtnl_link_stats64 *stats)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
-	struct i40e_ring *tx_ring, *rx_ring;
 	struct i40e_vsi *vsi = np->vsi;
 	struct rtnl_link_stats64 *vsi_stats = i40e_get_vsi_stats_struct(vsi);
+	struct i40e_ring *ring;
 	int i;
 
 	if (test_bit(__I40E_VSI_DOWN, vsi->state))
@@ -436,24 +436,26 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
 		u64 bytes, packets;
 		unsigned int start;
 
-		tx_ring = READ_ONCE(vsi->tx_rings[i]);
-		if (!tx_ring)
+		ring = READ_ONCE(vsi->tx_rings[i]);
+		if (!ring)
 			continue;
-		i40e_get_netdev_stats_struct_tx(tx_ring, stats);
+		i40e_get_netdev_stats_struct_tx(ring, stats);
 
-		rx_ring = &tx_ring[1];
+		if (i40e_enabled_xdp_vsi(vsi)) {
+			ring++;
+			i40e_get_netdev_stats_struct_tx(ring, stats);
+		}
 
+		ring++;
 		do {
-			start = u64_stats_fetch_begin_irq(&rx_ring->syncp);
-			packets = rx_ring->stats.packets;
-			bytes   = rx_ring->stats.bytes;
-		} while (u64_stats_fetch_retry_irq(&rx_ring->syncp, start));
+			start   = u64_stats_fetch_begin_irq(&ring->syncp);
+			packets = ring->stats.packets;
+			bytes   = ring->stats.bytes;
+		} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
 
 		stats->rx_packets += packets;
 		stats->rx_bytes   += bytes;
 
-		if (i40e_enabled_xdp_vsi(vsi))
-			i40e_get_netdev_stats_struct_tx(&rx_ring[1], stats);
 	}
 	rcu_read_unlock();
 
-- 
2.17.1

^ permalink raw reply related

* [net-next 12/15] i40evf: Don't enable vlan stripping when rx offload is turned on
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Patryk Małek, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Patryk Małek <patryk.malek@intel.com>

With current implementation of i40evf_set_features when user sets
any offload via ethtool we set I40EVF_FLAG_AQ_ENABLE_VLAN_STRIPPING
as a required aq which triggers driver to call
i40evf_enable_vlan_stripping. This shouldn't take place.
This patches fixes it by setting the flag only when VLAN offload
is turned on.

Signed-off-by: Patryk Małek <patryk.malek@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 07f187b5bcac..c7048cf484fb 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -3120,18 +3120,19 @@ static int i40evf_set_features(struct net_device *netdev,
 {
 	struct i40evf_adapter *adapter = netdev_priv(netdev);
 
-	/* Don't allow changing VLAN_RX flag when VLAN is set for VF
-	 * and return an error in this case
+	/* Don't allow changing VLAN_RX flag when adapter is not capable
+	 * of VLAN offload
 	 */
-	if (VLAN_ALLOWED(adapter)) {
+	if (!VLAN_ALLOWED(adapter)) {
+		if ((netdev->features ^ features) & NETIF_F_HW_VLAN_CTAG_RX)
+			return -EINVAL;
+	} else if ((netdev->features ^ features) & NETIF_F_HW_VLAN_CTAG_RX) {
 		if (features & NETIF_F_HW_VLAN_CTAG_RX)
 			adapter->aq_required |=
 				I40EVF_FLAG_AQ_ENABLE_VLAN_STRIPPING;
 		else
 			adapter->aq_required |=
 				I40EVF_FLAG_AQ_DISABLE_VLAN_STRIPPING;
-	} else if ((netdev->features ^ features) & NETIF_F_HW_VLAN_CTAG_RX) {
-		return -EINVAL;
 	}
 
 	return 0;
-- 
2.17.1

^ permalink raw reply related

* [net-next 05/15] i40evf: Validate the number of queues a PF sends
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem
  Cc: Paul M Stillwell Jr, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>

A PF can send any number of queues to the VF and the VF may not
be able to support that many. Check to see that the number of
queues is less than or equal to the max number of queues the
VF can have.

Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 .../ethernet/intel/i40evf/i40evf_virtchnl.c   | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 79b7be83b5c3..6579dabab78c 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -153,6 +153,32 @@ int i40evf_send_vf_config_msg(struct i40evf_adapter *adapter)
 					  NULL, 0);
 }
 
+/**
+ * i40evf_validate_num_queues
+ * @adapter: adapter structure
+ *
+ * Validate that the number of queues the PF has sent in
+ * VIRTCHNL_OP_GET_VF_RESOURCES is not larger than the VF can handle.
+ **/
+static void i40evf_validate_num_queues(struct i40evf_adapter *adapter)
+{
+	if (adapter->vf_res->num_queue_pairs > I40EVF_MAX_REQ_QUEUES) {
+		struct virtchnl_vsi_resource *vsi_res;
+		int i;
+
+		dev_info(&adapter->pdev->dev, "Received %d queues, but can only have a max of %d\n",
+			 adapter->vf_res->num_queue_pairs,
+			 I40EVF_MAX_REQ_QUEUES);
+		dev_info(&adapter->pdev->dev, "Fixing by reducing queues to %d\n",
+			 I40EVF_MAX_REQ_QUEUES);
+		adapter->vf_res->num_queue_pairs = I40EVF_MAX_REQ_QUEUES;
+		for (i = 0; i < adapter->vf_res->num_vsis; i++) {
+			vsi_res = &adapter->vf_res->vsi_res[i];
+			vsi_res->num_queue_pairs = I40EVF_MAX_REQ_QUEUES;
+		}
+	}
+}
+
 /**
  * i40evf_get_vf_config
  * @adapter: private adapter structure
@@ -195,6 +221,11 @@ int i40evf_get_vf_config(struct i40evf_adapter *adapter)
 	err = (i40e_status)le32_to_cpu(event.desc.cookie_low);
 	memcpy(adapter->vf_res, event.msg_buf, min(event.msg_len, len));
 
+	/* some PFs send more queues than we should have so validate that
+	 * we aren't getting too many queues
+	 */
+	if (!err)
+		i40evf_validate_num_queues(adapter);
 	i40e_vf_parse_hw_config(hw, adapter->vf_res);
 out_alloc:
 	kfree(event.msg_buf);
@@ -1329,6 +1360,7 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
 			  I40E_MAX_VF_VSI *
 			  sizeof(struct virtchnl_vsi_resource);
 		memcpy(adapter->vf_res, msg, min(msglen, len));
+		i40evf_validate_num_queues(adapter);
 		i40e_vf_parse_hw_config(&adapter->hw, adapter->vf_res);
 		if (is_zero_ether_addr(adapter->hw.mac.addr)) {
 			/* restore current mac address */
-- 
2.17.1

^ permalink raw reply related

* [net-next 13/15] i40e: hold the rtnl lock on clearing interrupt scheme
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Patryk Małek, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Patryk Małek <patryk.malek@intel.com>

Hold the rtnl lock when we're clearing interrupt scheme
in i40e_shutdown and in i40e_remove.

Signed-off-by: Patryk Małek <patryk.malek@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c30feb27e1c0..112245f32d7d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14182,6 +14182,7 @@ static void i40e_remove(struct pci_dev *pdev)
 	mutex_destroy(&hw->aq.asq_mutex);
 
 	/* Clear all dynamic memory lists of rings, q_vectors, and VSIs */
+	rtnl_lock();
 	i40e_clear_interrupt_scheme(pf);
 	for (i = 0; i < pf->num_alloc_vsi; i++) {
 		if (pf->vsi[i]) {
@@ -14190,6 +14191,7 @@ static void i40e_remove(struct pci_dev *pdev)
 			pf->vsi[i] = NULL;
 		}
 	}
+	rtnl_unlock();
 
 	for (i = 0; i < I40E_MAX_VEB; i++) {
 		kfree(pf->veb[i]);
@@ -14401,7 +14403,13 @@ static void i40e_shutdown(struct pci_dev *pdev)
 	wr32(hw, I40E_PFPM_WUFC,
 	     (pf->wol_en ? I40E_PFPM_WUFC_MAG_MASK : 0));
 
+	/* Since we're going to destroy queues during the
+	 * i40e_clear_interrupt_scheme() we should hold the RTNL lock for this
+	 * whole section
+	 */
+	rtnl_lock();
 	i40e_clear_interrupt_scheme(pf);
+	rtnl_unlock();
 
 	if (system_state == SYSTEM_POWER_OFF) {
 		pci_wake_from_d3(pdev, pf->wol_en);
-- 
2.17.1

^ permalink raw reply related

* [net-next 06/15] i40e: use correct length for strncpy
From: Jeff Kirsher @ 2018-08-29 22:48 UTC (permalink / raw)
  To: davem; +Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180829224834.3141-1-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

Caught by GCC 8. When we provide a length for strncpy, we should not
include the terminating null. So we must tell it one less than the size
of the destination buffer.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ptp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 35f2866b38c6..1199f0502d6d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -694,7 +694,8 @@ static long i40e_ptp_create_clock(struct i40e_pf *pf)
 	if (!IS_ERR_OR_NULL(pf->ptp_clock))
 		return 0;
 
-	strncpy(pf->ptp_caps.name, i40e_driver_name, sizeof(pf->ptp_caps.name));
+	strncpy(pf->ptp_caps.name, i40e_driver_name,
+		sizeof(pf->ptp_caps.name) - 1);
 	pf->ptp_caps.owner = THIS_MODULE;
 	pf->ptp_caps.max_adj = 999999999;
 	pf->ptp_caps.n_ext_ts = 0;
-- 
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