Netdev List
 help / color / mirror / Atom feed
* [PATCH net v1 2/3] tipc: improve error validations for sockets in CONNECTING state
From: Parthasarathy Bhuvaragan @ 2017-04-26  8:05 UTC (permalink / raw)
  To: netdev; +Cc: tipc-discussion
In-Reply-To: <1493193902-18332-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>

Until now, the checks for sockets in CONNECTING state was based on
the assumption that the incoming message was always from the
peer's accepted data socket.

However an application using a non-blocking socket sends an implicit
connect, this socket which is in CONNECTING state can receive error
messages from the peer's listening socket. As we discard these
messages, the application socket hangs as there due to inactivity.
In addition to this, there are other places where we process errors
but do not notify the user.

In this commit, we process such incoming error messages and notify
our users about them using sk_state_change().

Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/socket.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 3b8df510a80c..38c367f6ced4 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1259,7 +1259,10 @@ static int tipc_wait_for_rcvmsg(struct socket *sock, long *timeop)
 	struct sock *sk = sock->sk;
 	DEFINE_WAIT(wait);
 	long timeo = *timeop;
-	int err;
+	int err = sock_error(sk);
+
+	if (err)
+		return err;
 
 	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
@@ -1281,6 +1284,10 @@ static int tipc_wait_for_rcvmsg(struct socket *sock, long *timeop)
 		err = sock_intr_errno(timeo);
 		if (signal_pending(current))
 			break;
+
+		err = sock_error(sk);
+		if (err)
+			break;
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	*timeop = timeo;
@@ -1551,6 +1558,8 @@ static bool filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
 	struct sock *sk = &tsk->sk;
 	struct net *net = sock_net(sk);
 	struct tipc_msg *hdr = buf_msg(skb);
+	u32 pport = msg_origport(hdr);
+	u32 pnode = msg_orignode(hdr);
 
 	if (unlikely(msg_mcast(hdr)))
 		return false;
@@ -1558,18 +1567,28 @@ static bool filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
 	switch (sk->sk_state) {
 	case TIPC_CONNECTING:
 		/* Accept only ACK or NACK message */
-		if (unlikely(!msg_connected(hdr)))
-			return false;
+		if (unlikely(!msg_connected(hdr))) {
+			if (pport != tsk_peer_port(tsk) ||
+			    pnode != tsk_peer_node(tsk))
+				return false;
+
+			tipc_set_sk_state(sk, TIPC_DISCONNECTING);
+			sk->sk_err = ECONNREFUSED;
+			sk->sk_state_change(sk);
+			return true;
+		}
 
 		if (unlikely(msg_errcode(hdr))) {
 			tipc_set_sk_state(sk, TIPC_DISCONNECTING);
 			sk->sk_err = ECONNREFUSED;
+			sk->sk_state_change(sk);
 			return true;
 		}
 
 		if (unlikely(!msg_isdata(hdr))) {
 			tipc_set_sk_state(sk, TIPC_DISCONNECTING);
 			sk->sk_err = EINVAL;
+			sk->sk_state_change(sk);
 			return true;
 		}
 
-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related

* [PATCH net v1 1/3] tipc: Fix missing connection request handling
From: Parthasarathy Bhuvaragan @ 2017-04-26  8:05 UTC (permalink / raw)
  To: netdev; +Cc: tipc-discussion
In-Reply-To: <1493193902-18332-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>

In filter_connect, we use waitqueue_active() to check for any
connections to wakeup. But waitqueue_active() is missing memory
barriers while accessing the critical sections, leading to
inconsistent results.

In this commit, we replace this with an SMP safe wq_has_sleeper()
using the generic socket callback sk_data_ready().

Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 566906795c8c..3b8df510a80c 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1581,8 +1581,7 @@ static bool filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
 			return true;
 
 		/* If empty 'ACK-' message, wake up sleeping connect() */
-		if (waitqueue_active(sk_sleep(sk)))
-			wake_up_interruptible(sk_sleep(sk));
+		sk->sk_data_ready(sk);
 
 		/* 'ACK-' message is neither accepted nor rejected: */
 		msg_set_dest_droppable(hdr, 1);
-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related

* [PATCH net v1 0/3] tipc: fix hanging socket connections
From: Parthasarathy Bhuvaragan @ 2017-04-26  8:04 UTC (permalink / raw)
  To: netdev; +Cc: tipc-discussion

This patch series contains fixes for the socket layer to
prevent hanging / stale connections.

Parthasarathy Bhuvaragan (3):
  tipc: Fix missing connection request handling
  tipc: improve error validations for sockets in CONNECTING state
  tipc: close the connection if protocol messages contain errors

 net/tipc/socket.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply

* Re: [PATCH v2 02/21] libiscsi: Add an internal error code
From: Christoph Hellwig @ 2017-04-26  7:48 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: dri-devel, Stephen Bates, dm-devel, target-devel,
	Christoph Hellwig, devel, James E.J. Bottomley, linux-scsi,
	linux-nvdimm, linux-rdma, Sumit Semwal, Ross Zwisler, open-iscsi,
	linux-media, intel-gfx, sparmaintainer, linux-raid, Dan Williams,
	megaraidlinux.pdl, Jens Axboe, Martin K. Petersen, netdev,
	Matthew Wilcox, linux-mmc, linux-kernel, linux-crypto,
	Greg Kroah-Hartman
In-Reply-To: <1493144468-22493-3-git-send-email-logang@deltatee.com>

On Tue, Apr 25, 2017 at 12:20:49PM -0600, Logan Gunthorpe wrote:
> This is a prep patch to add a new error code to libiscsi. We want to
> rework some kmap calls to be able to fail. When we do, we'd like to
> use this error code.

The kmap case in iscsi_tcp_segment_map can already fail.  Please add
handling of that failure to this patch, and we should get it merged
ASAP.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH] IB/IPoIB: Check the headroom size
From: Paolo Abeni @ 2017-04-26  7:46 UTC (permalink / raw)
  To: Or Gerlitz, Doug Ledford
  Cc: Erez Shitrit, Honggang LI, Erez Shitrit,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Netdev List, David Miller
In-Reply-To: <CAJ3xEMjqkJrKi+6ronuPBn2P6y8p6sdhVppDzFtMwQrhL13bzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, 2017-04-26 at 00:50 +0300, Or Gerlitz wrote:
> so maybe @ least for the time being, we should be picking Hong's patch
> with proper change log and without the giant stack dump till we have
> something better. If you agree, can you do the re-write of the change
> log?

I think that Hong's patch is following the correct way to fix the
issue: ipoib_hard_header() can't assume that the skb headroom is at
least IPOIB_HARD_LEN bytes, as wrongly implied by commit fc791b633515
(my fault, I'm sorry).

Perhaps we can make the code a little more robust with something
alongside the following (only compile tested):
---
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index d1d3fb7..d53d2e1 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -1160,6 +1160,11 @@ static int ipoib_hard_header(struct sk_buff *skb,
                             const void *daddr, const void *saddr, unsigned len)
 {
        struct ipoib_header *header;
+       int ret;
+
+       ret = skb_cow_head(skb, IPOIB_HARD_LEN);
+       if (ret)
+               return ret;
 
        header = (struct ipoib_header *) skb_push(skb, sizeof *header);
---

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Christoph Hellwig @ 2017-04-26  7:44 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	target-devel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Christoph Hellwig,
	Martin K. Petersen, James E.J. Bottomley, Jens Axboe,
	Greg Kroah-Hartman, Dan Williams, Ross Zwisler, Matthew Wilcox,
	Sumit Semwal, Stephen Bates <s
In-Reply-To: <1493144468-22493-2-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>

On Tue, Apr 25, 2017 at 12:20:48PM -0600, Logan Gunthorpe wrote:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
> 
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)

I think we'll at least need a draft of those to make sense of these
patches.  Otherwise they just look very clumsy.

> + *   Use this function to map a page in the scatterlist at the specified
> + *   offset. sg->offset is already added for you. Note: the semantics of
> + *   this function are that it may fail. Thus, its output should be checked
> + *   with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + *   in the mapped page is returned.
> + *
> + *   Flags can be any of:
> + *	* SG_KMAP		- Use kmap to create the mapping
> + *	* SG_KMAP_ATOMIC	- Use kmap_atomic to map the page atommically.
> + *				  Thus, the rules of that function apply: the
> + *				  cpu may not sleep until it is unmaped.
> + *	* SG_MAP_MUST_NOT_FAIL	- Indicate that sg_map must not fail.
> + *				  If it does, it will issue a BUG_ON instead.
> + *				  This is intended for legacy code only, it
> + *				  is not to be used in new code.

I'm sorry but this API is just a trainwreck.  Right now we have the
nice little kmap_atomic API, which never fails and has a very nice
calling convention where we just pass back the return address, but does
not support sleeping inside the critical section.

And kmap, whіch may fail and requires the original page to be passed
back.  Anything that mixes these two concepts up is simply a non-starter.

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function
From: Roger Pau Monné @ 2017-04-26  7:37 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Boris Ostrovsky, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Juergen Gross, Julien Grall,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, netdev-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <1493144468-22493-16-git-send-email-logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>

On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
> Straightforward conversion to the new helper, except due to the lack
> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
> certain cases in the future.
> 
> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
> Cc: Boris Ostrovsky <boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: "Roger Pau Monné" <roger.pau-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/block/xen-blkfront.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 3945963..ed62175 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
>  		BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>  
>  		if (setup.need_copy) {
> -			setup.bvec_off = sg->offset;
> -			setup.bvec_data = kmap_atomic(sg_page(sg));
> +			setup.bvec_off = 0;
> +			setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
> +						 SG_MAP_MUST_NOT_FAIL);

I assume that sg_map already adds sg->offset to the address?

Also wondering whether we can get rid of bvec_off and just increment bvec_data,
adding Julien who IIRC added this code.

>  		}
>  
>  		gnttab_foreach_grant_in_range(sg_page(sg),
> @@ -827,7 +828,7 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
>  					      &setup);
>  
>  		if (setup.need_copy)
> -			kunmap_atomic(setup.bvec_data);
> +			sg_unmap(sg, setup.bvec_data, 0, SG_KMAP_ATOMIC);
>  	}
>  	if (setup.segments)
>  		kunmap_atomic(setup.segments);
> @@ -1053,7 +1054,7 @@ static int xen_translate_vdev(int vdevice, int *minor, unsigned int *offset)
>  		case XEN_SCSI_DISK5_MAJOR:
>  		case XEN_SCSI_DISK6_MAJOR:
>  		case XEN_SCSI_DISK7_MAJOR:
> -			*offset = (*minor / PARTS_PER_DISK) + 
> +			*offset = (*minor / PARTS_PER_DISK) +
>  				((major - XEN_SCSI_DISK1_MAJOR + 1) * 16) +
>  				EMULATED_SD_DISK_NAME_OFFSET;
>  			*minor = *minor +
> @@ -1068,7 +1069,7 @@ static int xen_translate_vdev(int vdevice, int *minor, unsigned int *offset)
>  		case XEN_SCSI_DISK13_MAJOR:
>  		case XEN_SCSI_DISK14_MAJOR:
>  		case XEN_SCSI_DISK15_MAJOR:
> -			*offset = (*minor / PARTS_PER_DISK) + 
> +			*offset = (*minor / PARTS_PER_DISK) +
>  				((major - XEN_SCSI_DISK8_MAJOR + 8) * 16) +
>  				EMULATED_SD_DISK_NAME_OFFSET;
>  			*minor = *minor +
> @@ -1119,7 +1120,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
>  	if (!VDEV_IS_EXTENDED(info->vdevice)) {
>  		err = xen_translate_vdev(info->vdevice, &minor, &offset);
>  		if (err)
> -			return err;		
> +			return err;

Cosmetic changes should go in a separate patch please.

Roger.

^ permalink raw reply

* Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
From: Gao Feng @ 2017-04-26  7:28 UTC (permalink / raw)
  To: 'Sven Eckelmann'
  Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, a,
	'Gao Feng', davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <2857108.moq831zFsU@bentobox>

> From: Sven Eckelmann [mailto:sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org]
> Sent: Wednesday, April 26, 2017 3:17 PM
> On Mittwoch, 26. April 2017 14:44:24 CEST Gao Feng wrote:
> [...]
> > I get it now, thanks.
> [...]
> > BTW, I think although the batadv_softif_create is legacy, we should
> > fix it when it still exists :)
> 
> I didn't meant that we should not fix it. I just said that it looks to me
like the fix
> should look different to ensure that it actually fixes the sysfs and rtnl
link
> implementation for the batadv interface creation. Right now the ndo_uninit
> (when it would be set by batadv) is called in the netdev core functions
when an
> error happens during the registration. This is not the case for the
destructor.

Thanks your answer.
I assumed the destructor is not for this case before.. 

> Your patch would not change it. It therefore looks like you simply have to
move
> the current destructor (without the free_netdev) to ndo_uninit and change
the
> destructor to free_netdev.

Yes, that patch didn't touch badman-adv. Because current badman-adv doesn't
support newlink now.
It would be good that cleanup the resource in ndo_uninit routine.

Best Regards
Feng
 
> 
> The batadv ops doesn't have a newlink function. It will therefore use the
> register_netdevice code path which calls free_netdev on failures. The
extra
> cleanup you've added in
> https://www.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg165253.html can
> therefore not work for batman-adv. Actually, it is not touching anything
> batman-adv related. The suggestion to change the register_netdevice ->
> free_netdev part in rtnl_newlink was new in the reply to the batadv
discussion.
> It is therefore still an open discussion how it is correctly fixed.
> 
> Kind regards,
> 	Sven

^ permalink raw reply

* [PATCH net-next v1] net: ipv6: make sure multicast packets are not forwarded beyond the different scopes
From: Donatas Abraitis @ 2017-04-26  7:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, stable, donatas.abraitis

	    RFC4291 2.7 Routers must not forward any multicast packets
	    beyond of the scope indicated by the scop field in the
	    destination multicast address.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
---
 net/ipv6/ip6_input.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 9ee208a..a834797 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -165,6 +165,14 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
 	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 0)
 		goto err;
 
+	/* RFC4291 2.7
+	 * Routers must not forward any multicast packets beyond of the scope
+	 * indicated by the scop field in the destination multicast address.
+	*/
+	if (ipv6_addr_is_multicast(&hdr->daddr) &&
+	    IPV6_ADDR_MC_SCOPE(&hdr->daddr) != IPV6_ADDR_MC_SCOPE(&hdr->saddr)
+	        goto err;
+
 	/*
 	 * RFC4291 2.7
 	 * Multicast addresses must not be used as source addresses in IPv6
-- 
2.10.0

^ permalink raw reply related

* Re: [RFC 1/4] netlink: make extended ACK setting NULL-friendly
From: Johannes Berg @ 2017-04-26  7:17 UTC (permalink / raw)
  To: Jakub Kicinski, daniel
  Cc: netdev, davem, dsa, alexei.starovoitov, bblanco, john.fastabend,
	oss-drivers
In-Reply-To: <20170425135306.0ff1db27@cakuba.netronome.com>

On Tue, 2017-04-25 at 13:53 -0700, Jakub Kicinski wrote:
> On Tue, 25 Apr 2017 10:13:34 +0200, Johannes Berg wrote:
> > On Tue, 2017-04-25 at 01:06 -0700, Jakub Kicinski wrote:
> > 
> > > +#define NL_SET_ERR_MSG(extack, msg) do {		\
> > > +	struct netlink_ext_ack *_extack = (extack);	\
> > > +	static const char _msg[] = (msg);		\
> > > +							\
> > > +	if (_extack)					\
> > > +		_extack->_msg = _msg;			\
> > > +	else						\
> > > +		pr_info("%s\n", _msg);			\
> > >  } while (0)  

> I'm leaning towards dropping the else clause and never printing, that
> will add an incentive for people to convert more paths to provide the
> ext ack.  Any thoughts on that?

Personally, I'm happy with that.

johannes

^ permalink raw reply

* Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
From: Sven Eckelmann @ 2017-04-26  7:16 UTC (permalink / raw)
  To: Gao Feng
  Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, a,
	'Gao Feng', davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <002101d2be58$8bcaaa00$a35ffe00$@foxmail.com>

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Mittwoch, 26. April 2017 14:44:24 CEST Gao Feng wrote:
[...]
> I get it now, thanks.
[...]
> BTW, I think although the batadv_softif_create is legacy, we should fix it
> when it still exists :)

I didn't meant that we should not fix it. I just said that it looks to me like 
the fix should look different to ensure that it actually fixes the sysfs and 
rtnl link implementation for the batadv interface creation. Right now the 
ndo_uninit (when it would be set by batadv) is called in the netdev core 
functions when an error happens during the registration. This is not the case 
for the destructor. Your patch would not change it. It therefore looks like 
you simply have to move the current destructor (without the free_netdev) to 
ndo_uninit and change the destructor to free_netdev.

The batadv ops doesn't have a newlink function. It will therefore use the 
register_netdevice code path which calls free_netdev on failures. The extra 
cleanup you've added in
 https://www.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg165253.html can 
therefore not work for batman-adv. Actually, it is not touching anything 
batman-adv related. The suggestion to change the register_netdevice -> 
free_netdev part in rtnl_newlink was new in the reply to the batadv 
discussion. It is therefore still an open discussion how it is correctly 
fixed.

Kind regards,
	Sven


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [iproute2] iplink: add support for IFLA_CARRIER attribute
From: Zhang Shengju @ 2017-04-26  7:08 UTC (permalink / raw)
  To: netdev

Add support to set IFLA_CARRIER attribute.

Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
 ip/iplink.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/ip/iplink.c b/ip/iplink.c
index 866ad72..263bfdd 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -72,6 +72,7 @@ void iplink_usage(void)
 		"	                  [ allmulticast { on | off } ]\n"
 		"	                  [ promisc { on | off } ]\n"
 		"	                  [ trailers { on | off } ]\n"
+		"	                  [ carrier { on | off } ]\n"
 		"	                  [ txqueuelen PACKETS ]\n"
 		"	                  [ name NEWNAME ]\n"
 		"	                  [ address LLADDR ]\n"
@@ -673,6 +674,17 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				req->i.ifi_flags |= IFF_NOARP;
 			else
 				return on_off("arp", *argv);
+		} else if (strcmp(*argv, "carrier") == 0) {
+			int carrier;
+			NEXT_ARG();
+			if (strcmp(*argv, "on") == 0)
+				carrier = 1;
+			else if (strcmp(*argv, "off") == 0)
+				carrier = 0;
+			else
+				return on_off("carrier", *argv);
+
+			addattr8(&req->n, sizeof(*req), IFLA_CARRIER, carrier);
 		} else if (strcmp(*argv, "vf") == 0) {
 			struct rtattr *vflist;
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
From: Gao Feng @ 2017-04-26  6:44 UTC (permalink / raw)
  To: 'Sven Eckelmann'
  Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, a,
	'Gao Feng', davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <3994535.Z9kqCnttI2@bentobox>

> From: Sven Eckelmann [mailto:sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org]
> Sent: Wednesday, April 26, 2017 1:58 PM
> On Mittwoch, 26. April 2017 08:41:30 CEST Gao Feng wrote:
> > On Dienstag, 25. April 2017 20:03:20 CEST gfree.wind-H32Fclmsjq1BDgjK7y7TUQ@public.gmane.org wrote:
> > > From: Gao Feng <fgao-KlmEoCYek3zQT0dZR+AlfA@public.gmane.org>
> > >
> > > Because the func batadv_softif_init_late allocate some resources and
> > > it would be invoked in register_netdevice. So we need to invoke the
> > > func batadv_softif_free instead of free_netdev to cleanup when fail
> > > to register_netdevice.
> >
> > I could be wrong, but shouldn't the destructor be replaced with
> > free_netdevice and the batadv_softif_free (without the free_netdev)
> > used as ndo_uninit? The line you've changed should then be kept as
> free_netdevice.
> >
> > At least this seems to be important when using rtnl_newlink() instead
> > of the legacy sysfs netdev stuff from batman-adv. rtnl_newlink() would
> > also only call free_netdevice and thus also not run
> > batadv_softif_free. This seems to be only fixable by calling ndo_uninit.
> >
> > Sorry, I don't get you.
> > The net_dev is created in this func batadv_softif_create.
> > Why couldn't invoke batadv_softif_free to cleanup when fail to
> > register_netdevice?
> >
> 
> Because it is the legacy way to create the batadv interfaces and there is
a
> "new" one. The new way is to use rtnl link (see batadv_link_ops).
> 
> The rtnl linke (rtnl_newlink) would not benefit from your fix and
therefore still
> show the old behavior. I think a different fix is necessary to solve the
problem
> for both ways to create an batadv interface.
> 
> Kind regards,
> 	Sven

I get it now, thanks.
Actually I sent another patch about the memleaks when invoke newlink and
fail to register_netdev.
You could refer to it by
https://www.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg165253.html.

In this patch, I add the cleanup when fail to register_netdev.
It would be more simple if modify the rtnl_newlink and invoke the destructor
of netdev when failed.
Like the following codes.
	if (ops->newlink) {
		err = ops->newlink(link_net ? : net, dev, tb, data);
		if (err < 0) {
			if (dev->reg_state == NETREG_UNINITIALIZED)
				if (dev->destructor)
					dev->destructor(dev)
				else
					free_netdev(dev);
				goto out;
			}
		} else {
			err = register_netdevice(dev);
			if (err < 0) {
				if (dev->destructor)
					dev->destructor(dev);
				else
					free_netdev(dev);
				goto out;
			}
	}

But I don't know if it breaks the design newlink and destructor.

BTW, I think although the batadv_softif_create is legacy, we should fix it
when it still exists :)

Regards
Feng

^ permalink raw reply

* [PATCH net v3 3/3] net: hns: fixed bug that skb used after kfree
From: Yankejian @ 2017-04-26  7:00 UTC (permalink / raw)
  To: davem, salil.mehta, yisen.zhuang, matthias.bgg, yankejian,
	lipeng321, zhouhuiru, huangdaode
  Cc: netdev, linuxarm
In-Reply-To: <1493190022-91343-1-git-send-email-yankejian@huawei.com>

From: lipeng <lipeng321@huawei.com>

There is KASAN warning which turn out it's a skb used after free:

    BUG: KASAN: use-after-free in hns_nic_net_xmit_hw+0x62c/0x940...
    [17659.112635]      alloc_debug_processing+0x18c/0x1a0
    [17659.117208]      __slab_alloc+0x52c/0x560
    [17659.120909]      kmem_cache_alloc_node+0xac/0x2c0
    [17659.125309]      __alloc_skb+0x6c/0x260
    [17659.128837]      tcp_send_ack+0x8c/0x280
    [17659.132449]      __tcp_ack_snd_check+0x9c/0xf0
    [17659.136587]      tcp_rcv_established+0x5a4/0xa70
    [17659.140899]      tcp_v4_do_rcv+0x27c/0x620
    [17659.144687]      tcp_prequeue_process+0x108/0x170
    [17659.149085]      tcp_recvmsg+0x940/0x1020
    [17659.152787]      inet_recvmsg+0x124/0x180
    [17659.156488]      sock_recvmsg+0x64/0x80
    [17659.160012]      SyS_recvfrom+0xd8/0x180
    [17659.163626]      __sys_trace_return+0x0/0x4
    [17659.167506] INFO: Freed in kfree_skbmem+0xa0/0xb0 age=23 cpu=1 pid=13
    [17659.174000]      free_debug_processing+0x1d4/0x2c0
    [17659.178486]      __slab_free+0x240/0x390
    [17659.182100]      kmem_cache_free+0x24c/0x270
    [17659.186062]      kfree_skbmem+0xa0/0xb0
    [17659.189587]      __kfree_skb+0x28/0x40
    [17659.193025]      napi_gro_receive+0x168/0x1c0
    [17659.197074]      hns_nic_rx_up_pro+0x58/0x90
    [17659.201038]      hns_nic_rx_poll_one+0x518/0xbc0
    [17659.205352]      hns_nic_common_poll+0x94/0x140
    [17659.209576]      net_rx_action+0x458/0x5e0
    [17659.213363]      __do_softirq+0x1b8/0x480
    [17659.217062]      run_ksoftirqd+0x64/0x80
    [17659.220679]      smpboot_thread_fn+0x224/0x310
    [17659.224821]      kthread+0x150/0x170
    [17659.228084]      ret_from_fork+0x10/0x40

    BUG: KASAN: use-after-free in hns_nic_net_xmit+0x8c/0xc0...
    [17751.080490]      __slab_alloc+0x52c/0x560
    [17751.084188]      kmem_cache_alloc+0x244/0x280
    [17751.088238]      __build_skb+0x40/0x150
    [17751.091764]      build_skb+0x28/0x100
    [17751.095115]      __alloc_rx_skb+0x94/0x150
    [17751.098900]      __napi_alloc_skb+0x34/0x90
    [17751.102776]      hns_nic_rx_poll_one+0x180/0xbc0
    [17751.107097]      hns_nic_common_poll+0x94/0x140
    [17751.111333]      net_rx_action+0x458/0x5e0
    [17751.115123]      __do_softirq+0x1b8/0x480
    [17751.118823]      run_ksoftirqd+0x64/0x80
    [17751.122437]      smpboot_thread_fn+0x224/0x310
    [17751.126575]      kthread+0x150/0x170
    [17751.129838]      ret_from_fork+0x10/0x40
    [17751.133454] INFO: Freed in kfree_skbmem+0xa0/0xb0 age=19 cpu=7 pid=43
    [17751.139951]      free_debug_processing+0x1d4/0x2c0
    [17751.144436]      __slab_free+0x240/0x390
    [17751.148051]      kmem_cache_free+0x24c/0x270
    [17751.152014]      kfree_skbmem+0xa0/0xb0
    [17751.155543]      __kfree_skb+0x28/0x40
    [17751.159022]      napi_gro_receive+0x168/0x1c0
    [17751.163074]      hns_nic_rx_up_pro+0x58/0x90
    [17751.167041]      hns_nic_rx_poll_one+0x518/0xbc0
    [17751.171358]      hns_nic_common_poll+0x94/0x140
    [17751.175585]      net_rx_action+0x458/0x5e0
    [17751.179373]      __do_softirq+0x1b8/0x480
    [17751.183076]      run_ksoftirqd+0x64/0x80
    [17751.186691]      smpboot_thread_fn+0x224/0x310
    [17751.190826]      kthread+0x150/0x170
    [17751.194093]      ret_from_fork+0x10/0x40

in drivers/net/ethernet/hisilicon/hns/hns_enet.c

    static netdev_tx_t hns_nic_net_xmit(struct sk_buff *skb,
                                        struct net_device *ndev)
    {
        struct hns_nic_priv *priv = netdev_priv(ndev);
        int ret;

        assert(skb->queue_mapping < ndev->ae_handle->q_num);
        ret = hns_nic_net_xmit_hw(ndev, skb,
                                  &tx_ring_data(priv, skb->queue_mapping));
        if (ret == NETDEV_TX_OK) {
                netif_trans_update(ndev);
                ndev->stats.tx_bytes += skb->len;
                ndev->stats.tx_packets++;
        }
         return (netdev_tx_t)ret;
    }

skb maybe freed in hns_nic_net_xmit_hw() and return NETDEV_TX_OK:

    out_err_tx_ok:

        dev_kfree_skb_any(skb);
        return NETDEV_TX_OK;

This patch fix this bug.

Reported-by: Jun He <hjat2005@huawei.com>
Signed-off-by: lipeng <lipeng321@huawei.com>
Reviewed-by: Yisen Zhuang <yisen.zhuang@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 22 ++++++++++------------
 drivers/net/ethernet/hisilicon/hns/hns_enet.h |  6 +++---
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index fca37e2..1e04df6 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -300,9 +300,9 @@ static void fill_tso_desc(struct hnae_ring *ring, void *priv,
 			     mtu);
 }
 
-int hns_nic_net_xmit_hw(struct net_device *ndev,
-			struct sk_buff *skb,
-			struct hns_nic_ring_data *ring_data)
+netdev_tx_t hns_nic_net_xmit_hw(struct net_device *ndev,
+				struct sk_buff *skb,
+				struct hns_nic_ring_data *ring_data)
 {
 	struct hns_nic_priv *priv = netdev_priv(ndev);
 	struct hnae_ring *ring = ring_data->ring;
@@ -361,6 +361,10 @@ int hns_nic_net_xmit_hw(struct net_device *ndev,
 	dev_queue = netdev_get_tx_queue(ndev, skb->queue_mapping);
 	netdev_tx_sent_queue(dev_queue, skb->len);
 
+	netif_trans_update(ndev);
+	ndev->stats.tx_bytes += skb->len;
+	ndev->stats.tx_packets++;
+
 	wmb(); /* commit all data before submit */
 	assert(skb->queue_mapping < priv->ae_handle->q_num);
 	hnae_queue_xmit(priv->ae_handle->qs[skb->queue_mapping], buf_num);
@@ -1474,17 +1478,11 @@ static netdev_tx_t hns_nic_net_xmit(struct sk_buff *skb,
 				    struct net_device *ndev)
 {
 	struct hns_nic_priv *priv = netdev_priv(ndev);
-	int ret;
 
 	assert(skb->queue_mapping < ndev->ae_handle->q_num);
-	ret = hns_nic_net_xmit_hw(ndev, skb,
-				  &tx_ring_data(priv, skb->queue_mapping));
-	if (ret == NETDEV_TX_OK) {
-		netif_trans_update(ndev);
-		ndev->stats.tx_bytes += skb->len;
-		ndev->stats.tx_packets++;
-	}
-	return (netdev_tx_t)ret;
+
+	return hns_nic_net_xmit_hw(ndev, skb,
+				   &tx_ring_data(priv, skb->queue_mapping));
 }
 
 static int hns_nic_change_mtu(struct net_device *ndev, int new_mtu)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.h b/drivers/net/ethernet/hisilicon/hns/hns_enet.h
index 5b412de..7bc6a6e 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.h
@@ -91,8 +91,8 @@ struct hns_nic_priv {
 void hns_nic_net_reset(struct net_device *ndev);
 void hns_nic_net_reinit(struct net_device *netdev);
 int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h);
-int hns_nic_net_xmit_hw(struct net_device *ndev,
-			struct sk_buff *skb,
-			struct hns_nic_ring_data *ring_data);
+netdev_tx_t hns_nic_net_xmit_hw(struct net_device *ndev,
+				struct sk_buff *skb,
+				struct hns_nic_ring_data *ring_data);
 
 #endif	/**__HNS_ENET_H */
-- 
1.9.1

^ permalink raw reply related

* [PATCH net v3 2/3] net: hns: support deferred probe when no mdio
From: Yankejian @ 2017-04-26  7:00 UTC (permalink / raw)
  To: davem, salil.mehta, yisen.zhuang, matthias.bgg, yankejian,
	lipeng321, zhouhuiru, huangdaode
  Cc: netdev, linuxarm
In-Reply-To: <1493190022-91343-1-git-send-email-yankejian@huawei.com>

From: lipeng <lipeng321@huawei.com>

In the hip06 and hip07 SoCs, phy connect to mdio bus.The mdio
module is probed with module_init, and, as such,
is not guaranteed to probe before the HNS driver. So we need
to support deferred probe.

We check for probe deferral in the mac init, so we not init DSAF
when there is no mdio, and free all resource, to later learn that
we need to defer the probe.

Signed-off-by: lipeng <lipeng321@huawei.com>
Reviewed-by: Yisen Zhuang <yisen.zhuang@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 39 +++++++++++++++++++----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index bdd8cdd..8c00e09 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -735,6 +735,8 @@ static int hns_mac_init_ex(struct hns_mac_cb *mac_cb)
 	rc = phy_device_register(phy);
 	if (rc) {
 		phy_device_free(phy);
+		dev_err(&mdio->dev, "registered phy fail at address %i\n",
+			addr);
 		return -ENODEV;
 	}
 
@@ -745,7 +747,7 @@ static int hns_mac_init_ex(struct hns_mac_cb *mac_cb)
 	return 0;
 }
 
-static void hns_mac_register_phy(struct hns_mac_cb *mac_cb)
+static int hns_mac_register_phy(struct hns_mac_cb *mac_cb)
 {
 	struct acpi_reference_args args;
 	struct platform_device *pdev;
@@ -755,24 +757,39 @@ static void hns_mac_register_phy(struct hns_mac_cb *mac_cb)
 
 	/* Loop over the child nodes and register a phy_device for each one */
 	if (!to_acpi_device_node(mac_cb->fw_port))
-		return;
+		return -ENODEV;
 
 	rc = acpi_node_get_property_reference(
 			mac_cb->fw_port, "mdio-node", 0, &args);
 	if (rc)
-		return;
+		return rc;
 
 	addr = hns_mac_phy_parse_addr(mac_cb->dev, mac_cb->fw_port);
 	if (addr < 0)
-		return;
+		return addr;
 
 	/* dev address in adev */
 	pdev = hns_dsaf_find_platform_device(acpi_fwnode_handle(args.adev));
+	if (!pdev) {
+		dev_err(mac_cb->dev, "mac%d mdio pdev is NULL\n",
+			mac_cb->mac_id);
+		return  -EINVAL;
+	}
+
 	mii_bus = platform_get_drvdata(pdev);
+	if (!mii_bus) {
+		dev_err(mac_cb->dev,
+			"mac%d mdio is NULL, dsaf will probe again later\n",
+			mac_cb->mac_id);
+		return -EPROBE_DEFER;
+	}
+
 	rc = hns_mac_register_phydev(mii_bus, mac_cb, addr);
 	if (!rc)
 		dev_dbg(mac_cb->dev, "mac%d register phy addr:%d\n",
 			mac_cb->mac_id, addr);
+
+	return rc;
 }
 
 #define MAC_MEDIA_TYPE_MAX_LEN		16
@@ -793,7 +810,7 @@ static void hns_mac_register_phy(struct hns_mac_cb *mac_cb)
  *@np:device node
  * return: 0 --success, negative --fail
  */
-static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
+static int hns_mac_get_info(struct hns_mac_cb *mac_cb)
 {
 	struct device_node *np;
 	struct regmap *syscon;
@@ -903,7 +920,15 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
 			}
 		}
 	} else if (is_acpi_node(mac_cb->fw_port)) {
-		hns_mac_register_phy(mac_cb);
+		ret = hns_mac_register_phy(mac_cb);
+		/*
+		 * Mac can work well if there is phy or not.If the port don't
+		 * connect with phy, the return value will be ignored. Only
+		 * when there is phy but can't find mdio bus, the return value
+		 * will be handled.
+		 */
+		if (ret == -EPROBE_DEFER)
+			return ret;
 	} else {
 		dev_err(mac_cb->dev, "mac%d cannot find phy node\n",
 			mac_cb->mac_id);
@@ -1065,6 +1090,7 @@ int hns_mac_init(struct dsaf_device *dsaf_dev)
 			dsaf_dev->mac_cb[port_id] = mac_cb;
 		}
 	}
+
 	/* init mac_cb for all port */
 	for (port_id = 0; port_id < max_port_num; port_id++) {
 		mac_cb = dsaf_dev->mac_cb[port_id];
@@ -1074,6 +1100,7 @@ int hns_mac_init(struct dsaf_device *dsaf_dev)
 		ret = hns_mac_get_cfg(dsaf_dev, mac_cb);
 		if (ret)
 			return ret;
+
 		ret = hns_mac_init_ex(mac_cb);
 		if (ret)
 			return ret;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net v3 0/3] net: hns: bug fix for HNS driver
From: Yankejian @ 2017-04-26  7:00 UTC (permalink / raw)
  To: davem, salil.mehta, yisen.zhuang, matthias.bgg, yankejian,
	lipeng321, zhouhuiru, huangdaode
  Cc: netdev, linuxarm

From: lipeng <lipeng321@huawei.com>

The first two patches [1/3] [2/3] of this serie add support defered
dsaf probe when mdio and mbigen module is not insmod. The third
patch [3/3] fixes a bug that skb still be used after freed, which will
cuase panic.

For more details, please refer to individual patch.

change log:
V2 -> V3:
1. Check return value when  platform_get_irq in hns_rcb_get_cfg;

V1 -> V2:
1. Return appropriate errno in hns_mac_register_phy;

lipeng (3):
  net: hns: support deferred probe when can not obtain irq
  net: hns: support deferred probe when no mdio
  net: hns: fixed bug that skb used after kfree

 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 39 +++++++++++++++++++----
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c |  4 ++-
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c |  8 ++++-
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h |  2 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c     | 22 ++++++-------
 drivers/net/ethernet/hisilicon/hns/hns_enet.h     |  6 ++--
 6 files changed, 57 insertions(+), 24 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH net v3 1/3] net: hns: support deferred probe when can not obtain irq
From: Yankejian @ 2017-04-26  7:00 UTC (permalink / raw)
  To: davem, salil.mehta, yisen.zhuang, matthias.bgg, yankejian,
	lipeng321, zhouhuiru, huangdaode
  Cc: netdev, linuxarm
In-Reply-To: <1493190022-91343-1-git-send-email-yankejian@huawei.com>

From: lipeng <lipeng321@huawei.com>

In the hip06 and hip07 SoCs, the interrupt lines from the
DSAF controllers are connected to mbigen hw module.
The mbigen module is probed with module_init, and, as such,
is not guaranteed to probe before the HNS driver. So we need
to support deferred probe.

We check for probe deferral in the hw layer probe, so we not
probe into the main layer and memories, etc., to later learn
that we need to defer the probe.

Signed-off-by: lipeng <lipeng321@huawei.com>
Reviewed-by: Yisen Zhuang <yisen.zhuang@huawei.com>
---
V2 -> V3:
1. Check return value when  platform_get_irq in hns_rcb_get_cfg;
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 4 +++-
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c | 8 +++++++-
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index 6ea8722..a41cf95 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -510,7 +510,9 @@ int hns_ppe_init(struct dsaf_device *dsaf_dev)
 
 		hns_ppe_get_cfg(dsaf_dev->ppe_common[i]);
 
-		hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
+		ret = hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
+		if (ret)
+			goto get_rcb_cfg_fail;
 	}
 
 	for (i = 0; i < HNS_PPE_COM_NUM; i++)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
index f0ed80d6..673a5d3 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
@@ -452,7 +452,7 @@ static int hns_rcb_get_base_irq_idx(struct rcb_common_cb *rcb_common)
  *hns_rcb_get_cfg - get rcb config
  *@rcb_common: rcb common device
  */
-void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
+int hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
 {
 	struct ring_pair_cb *ring_pair_cb;
 	u32 i;
@@ -477,10 +477,16 @@ void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
 		ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] =
 		is_ver1 ? platform_get_irq(pdev, base_irq_idx + i * 2 + 1) :
 			  platform_get_irq(pdev, base_irq_idx + i * 3);
+		if ((ring_pair_cb->virq[HNS_RCB_IRQ_IDX_TX] == -EPROBE_DEFER) ||
+		    (ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] == -EPROBE_DEFER))
+			return -EPROBE_DEFER;
+
 		ring_pair_cb->q.phy_base =
 			RCB_COMM_BASE_TO_RING_BASE(rcb_common->phy_base, i);
 		hns_rcb_ring_pair_get_cfg(ring_pair_cb);
 	}
+
+	return 0;
 }
 
 /**
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
index 99b4e1b..3d7b484 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
@@ -110,7 +110,7 @@ struct rcb_common_cb {
 void hns_rcb_common_free_cfg(struct dsaf_device *dsaf_dev, u32 comm_index);
 int hns_rcb_common_init_hw(struct rcb_common_cb *rcb_common);
 void hns_rcb_start(struct hnae_queue *q, u32 val);
-void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common);
+int hns_rcb_get_cfg(struct rcb_common_cb *rcb_common);
 void hns_rcb_get_queue_mode(enum dsaf_mode dsaf_mode,
 			    u16 *max_vfn, u16 *max_q_per_vf);
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next v8 2/3] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-04-26  6:19 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: davem, xiyou.wangcong, eric.dumazet, netdev
In-Reply-To: <4b7789f7-69e0-4764-7029-f6e15d6e7d69@mojatatu.com>

Tue, Apr 25, 2017 at 10:29:40PM CEST, jhs@mojatatu.com wrote:
>On 17-04-25 12:04 PM, Jiri Pirko wrote:
>> Tue, Apr 25, 2017 at 03:01:22PM CEST, jhs@mojatatu.com wrote:
>> > On 17-04-25 08:13 AM, Jiri Pirko wrote:
>> > > Tue, Apr 25, 2017 at 01:54:06PM CEST, jhs@mojatatu.com wrote:
>
>> > > > 
>> > > > +static inline bool tca_flags_valid(u32 act_flags)
>> > > > +{
>> > > > +	u32 invalid_flags_mask  = ~VALID_TCA_FLAGS;
>> > > > +
>> > > > +	if (act_flags & invalid_flags_mask)
>> > > > +		return false;
>> > > 
>> > > I don't see how this resolves anything. VALID_TCA_FLAGS is set in stone
>> > > not going to change anytime in future, right?
>> > 
>> > Every time a new bit gets added VALID_TCA_FLAGS changes.
>> 
>> You mean flag that user can set? If that is the case, you are breaking
>> UAPI for newer app running on older kernel.
>> 
>
>Ok, let me try to explain with more clarity. The rules Iam
>trying to follow are:
>if i see any bit set that i dont understand I will reject.
>
>So lets in first kernel I have support for bit 0.
>My validation check is to make sure only bit 0 is set.
>The valid_flags currently then only constitutes bit 0.
>i.e
>If you set bit 2 or 3, the function above will reject and i return
>the error to the user.
>
>That is expected behavior correct?
>
>3 months down the road:
>I add two flags - bit 1 and 2.
>So now my valid_flags changes to bits 1, 2 and 0.
>
>The function above will now return true for bits 0-2 but
>will reject if you set bit 3.
>
>That is expected behavior, correct?

The same app compiled against new kernel with bits (0, 1, 2) will run with
this kernel good. But if you run it with older kernel, the kernel (0)
would refuse. Is that ok?



>
>On u32/16/8:
>I am choosing u32 so it allows me to add more upto 32 bit flags.
>Not all 32 are needed today but it is better insurance.
>If I used u8 then the 24 of those 32 bits i dont use will be used
>as pads in the TLV. So it doesnt make sense for me to use a u8/16.

Jamal, note that I never suggested having more flags in a single attr.
Therefore I suggested u8 to carry a single flag.

You say that it has performance impact having 3 flag attrs in compare to
one bit flag attr. Could you please provide some numbers?

I expect that you will not be able to show the difference. And if there
is no difference in performance, your main argument goes away. And we
can do this in a nice, clear, TLV fashion.


>
>Does that make more sense?
>
>cheers,
>jamal

^ permalink raw reply

* Re: [PATCH net] net: batman-adv: Fix possible memleaks when fail to register_netdevice
From: Sven Eckelmann @ 2017-04-26  5:57 UTC (permalink / raw)
  To: Gao Feng
  Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, a,
	'Gao Feng', davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <002901d2be25$d9092bd0$8b1b8370$@foxmail.com>

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

On Mittwoch, 26. April 2017 08:41:30 CEST Gao Feng wrote:
> On Dienstag, 25. April 2017 20:03:20 CEST gfree.wind-H32Fclmsjq1BDgjK7y7TUQ@public.gmane.org wrote:
> > From: Gao Feng <fgao-KlmEoCYek3zQT0dZR+AlfA@public.gmane.org>
> > 
> > Because the func batadv_softif_init_late allocate some resources and
> > it would be invoked in register_netdevice. So we need to invoke the
> > func batadv_softif_free instead of free_netdev to cleanup when fail
> > to register_netdevice.
> 
> I could be wrong, but shouldn't the destructor be replaced with free_netdevice 
> and the batadv_softif_free (without the free_netdev) used as ndo_uninit? The 
> line you've changed should then be kept as free_netdevice.
> 
> At least this seems to be important when using rtnl_newlink() instead of the 
> legacy sysfs netdev stuff from batman-adv. rtnl_newlink() would also only call 
> free_netdevice and thus also not run batadv_softif_free. This seems to be only 
> fixable by calling ndo_uninit.
> 
> Sorry, I don't get you.
> The net_dev is created in this func batadv_softif_create.
> Why couldn't invoke batadv_softif_free to cleanup when fail to
> register_netdevice?
> 

Because it is the legacy way to create the batadv interfaces and there is a 
"new" one. The new way is to use rtnl link (see batadv_link_ops). 

The rtnl linke (rtnl_newlink) would not benefit from your fix and therefore 
still show the old behavior. I think a different fix is necessary to solve the 
problem for both ways to create an batadv interface.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [Patch net 3/3] team: use a larger struct for mac address
From: Jiri Pirko @ 2017-04-26  5:40 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, andreyknvl
In-Reply-To: <1493183003-884-4-git-send-email-xiyou.wangcong@gmail.com>

Wed, Apr 26, 2017 at 07:03:23AM CEST, xiyou.wangcong@gmail.com wrote:
>IPv6 tunnels use sizeof(struct in6_addr) as dev->addr_len,
>but in many places especially bonding, we use struct sockaddr
>to copy and set mac addr, this could lead to stack out-of-bounds
>access.
>
>Fix it by using a larger address storage.
>
>Reported-by: Andrey Konovalov <andreyknvl@google.com>
>Cc: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>---
> drivers/net/team/team.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 85c0124..88878f1 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -60,10 +60,13 @@ static struct team_port *team_port_get_rtnl(const struct net_device *dev)
> static int __set_port_dev_addr(struct net_device *port_dev,
> 			       const unsigned char *dev_addr)
> {
>-	struct sockaddr addr;
>+	struct {
>+		unsigned short type;
>+		unsigned char addr[MAX_ADDR_LEN];
>+	} addr;

Wouldn't it make sense to define this struct somewhere in the core
headers?


> 
>-	memcpy(addr.sa_data, dev_addr, port_dev->addr_len);
>-	addr.sa_family = port_dev->type;
>+	memcpy(addr.addr, dev_addr, port_dev->addr_len);
>+	addr.type = port_dev->type;
> 	return dev_set_mac_address(port_dev, &addr);
> }
> 
>-- 
>2.5.5
>

^ permalink raw reply

* Re: hmmm...
From: Alexei Starovoitov @ 2017-04-26  5:31 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20170425.233820.1357836307906136732.davem@davemloft.net>

On 4/25/17 8:38 PM, David Miller wrote:
> From: Alexei Starovoitov <ast@fb.com>
> Date: Tue, 25 Apr 2017 19:56:06 -0700
>
>> On 4/25/17 6:52 PM, David Miller wrote:
>>>
>>> Alexei, I found something strange on my computer :-)
>>>
>>> [davem@localhost binutils]$ ./objdump -d x.o
>>
>> No way! :) I thought it will take weeks!
>> Ship it. Ship it. Ship it.
>> Cannot wait to pull.
>> This is awesome. Thanks a ton!
>
> Relax, it is in a very raw state still. :)

hehe, before replied i checked binutils repo...
hoping to find a miracle there :)

>> What is the mnemonic for 32-bit alu ?
>
> It is of the form "xxx32".  Here is opcodes table below.
>
> I think there are no formal mnenomics defined anywhere yet right?
> So just tell me what adjustments you want to make.
>
> const struct bpf_opcode bpf_opcodes[] = {
>   { "mov32",   BPF_OPC_ALU   | BPF_OPC_MOV  | BPF_OPC_X,     "1,2" },
>   { "mov",     BPF_OPC_ALU64 | BPF_OPC_MOV  | BPF_OPC_X,     "1,2" },
>   { "add32",   BPF_OPC_ALU   | BPF_OPC_ADD  | BPF_OPC_X,     "1,2" },

the very first bpf gen looked like canonical asm,
but then I found myself spending too much time
looking at ldb and ldd and thinking is it 8-byte
or 64-byte load?
jgt/jge/jsgt/sge was a stumbling block for me as well,
since it still takes me longer than necessary to disambiguate
into > vs >= and signed/unsigned
gcc bpf backend was done way before llvm and it had
quite confusing GT vs bpf GT vs x86 GT.
https://github.com/iovisor-obsolete/bpf_gcc/commit/ff47b3acd6d3c60900fab8cd93afd8106c49c8c3#diff-ee684c67b9329b734b7fb535d86a558dR452
gcc GTU == bpf GT == x86 GA
gcc GT == bpf SGT == x86 G
gcc GE == bpf SGE == x86 GE
I couldn't change bpf GT to GTU to match gcc, since it came
from classic bpf, so could only add SGT. S for Signed.
Through all this confusion I figured that the only output
not superhuman can quickly read is C-like output,
hence the final gcc output looked like:
https://github.com/iovisor-obsolete/bpf_gcc/commit/ff47b3acd6d3c60900fab8cd93afd8106c49c8c3#diff-0db71d60fd74b52befee102a27f0b4c4R292
+(define_insn "movdi_2mem"
+  [(set (match_operand:DI 0 "memory_operand" "=m,m")
+	(match_operand:DI 1 "int_reg_or_const_operand" "r,K"))]
+  ""
+  "@
+   BPF_INSN_ST(BPF_DW, %0, %1), // *(uint64*)(%0)=%1
+   BPF_INSN_ST_IMM(BPF_DW, %0, %1), // *(uint64*)(%0)=%1"
+[(set_attr "type" "store, store")])

yes, gcc was emitting C macros with C looking comments
into fake .s file that we #included into .c code to be loaded
into the kernel.
(that code is abandoned and probably doesn't compile anymore)

I made the verifier to emit C-like output to make it easier
for program writers to understand.

llvm asm until version 4.0 looked like:
         mov     r0, 1
         ldd     r4, 8(r1)
         ldd     r3, 0(r1)
         mov     r5, r3
         addi    r5, 14
         jgt     r5, r4 goto LBB0_3
which was causing confusion as well, so when I realized
that llvm can emit and parse any type of text, I switched
to emit verifier like asm code:
         r2 = *(u64 *)(r10 - 16)
         r5 = r2
         r5 += 34
         r0 = 2
         if r5 > r3 goto LBB0_1
which I think is way easier to read.

Though I think Daniel still prefers old classic bpf asm ;)

Anyway, back to the question...
since BFD and GCC are so much entrenched into canonical style
of asm code, I don't mind that gnu toolchain will be using it.

I like that you used 'dw' in 'ldxdw' instead of just 'd'
though 'x' can probably be dropped.

'x' should be added here instead:
  { "stb", BPF_OPC_ST    | BPF_OPC_MEM  | BPF_OPC_B, "[1+O],i" },
  { "stb", BPF_OPC_STX   | BPF_OPC_MEM  | BPF_OPC_B, "[1+O],2" },

I would use ld8/ld16/ld32/ld64 and st8/stx8/st16/stx16/...
but I spent too much time looking into asm and I'm biased.

Thanks again for working on it! This is major milestone.

^ permalink raw reply

* Re: IGMP on IPv6
From: Cong Wang @ 2017-04-26  5:24 UTC (permalink / raw)
  To: Murali Karicheri
  Cc: Hangbin Liu, open list:TI NETCP ETHERNET DRIVER, David Miller
In-Reply-To: <58FF7656.8050506@ti.com>

On Tue, Apr 25, 2017 at 9:16 AM, Murali Karicheri <m-karicheri2@ti.com> wrote:
> On 04/18/2017 06:37 PM, Cong Wang wrote:
>> Did you assign IPv4 and IPv6 addresses to the HSR master device?
>
> No. I just used IPv4. From the trace mld_ifc_timer_expire() -> mld_sendpack() -> ip6_output()
> do you know what is it trying to do? Is it some neighbor discovery message or something
> going over the lower interface instead of the hsr interface?
>

IPv6 is special here because it has link-local address configured
automatically for each device, so this mld_ifc_timer is armed
for link-local multicasts. See:


 464         /* Join interface-local all-node multicast group */
 465         ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
 466
 467         /* Join all-node multicast group */
 468         ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
 469
 470         /* Join all-router multicast group if forwarding is set */
 471         if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
 472                 ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);

^ permalink raw reply

* 47490 netdev
From: kelley @ 2017-04-26  5:19 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: 47176743700.zip --]
[-- Type: application/zip, Size: 1674 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 3/3] samples/bpf: check before defining offsetof
From: Alexander Alemayhu @ 2017-04-26  5:17 UTC (permalink / raw)
  To: David Laight
  Cc: 'Daniel Borkmann', netdev@vger.kernel.org, ast@fb.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFDAD25@AcuExch.aculab.com>

On Tue, Apr 25, 2017 at 02:27:16PM +0000, David Laight wrote:
> 
> Isn't the correct fix to include stddef.h ?
>
If you think it's the correct fix, please send a patch to netdev.

Thanks.

-- 
Mit freundlichen Grüßen

Alexander Alemayhu

^ permalink raw reply

* [Patch net 3/3] team: use a larger struct for mac address
From: Cong Wang @ 2017-04-26  5:03 UTC (permalink / raw)
  To: netdev; +Cc: andreyknvl, Cong Wang, Jiri Pirko
In-Reply-To: <1493183003-884-1-git-send-email-xiyou.wangcong@gmail.com>

IPv6 tunnels use sizeof(struct in6_addr) as dev->addr_len,
but in many places especially bonding, we use struct sockaddr
to copy and set mac addr, this could lead to stack out-of-bounds
access.

Fix it by using a larger address storage.

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 drivers/net/team/team.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 85c0124..88878f1 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -60,10 +60,13 @@ static struct team_port *team_port_get_rtnl(const struct net_device *dev)
 static int __set_port_dev_addr(struct net_device *port_dev,
 			       const unsigned char *dev_addr)
 {
-	struct sockaddr addr;
+	struct {
+		unsigned short type;
+		unsigned char addr[MAX_ADDR_LEN];
+	} addr;
 
-	memcpy(addr.sa_data, dev_addr, port_dev->addr_len);
-	addr.sa_family = port_dev->type;
+	memcpy(addr.addr, dev_addr, port_dev->addr_len);
+	addr.type = port_dev->type;
 	return dev_set_mac_address(port_dev, &addr);
 }
 
-- 
2.5.5

^ 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