Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] once: switch to new jump label API
From: Eric Biggers @ 2017-10-09 21:30 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, linux-kernel, Hannes Frederic Sowa, Jason Baron,
	Peter Zijlstra, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

Switch the DO_ONCE() macro from the deprecated jump label API to the new
one.  The new one is more readable, and for DO_ONCE() it also makes the
generated code more icache-friendly: now the one-time initialization
code is placed out-of-line at the jump target, rather than at the inline
fallthrough case.

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 include/linux/once.h | 6 +++---
 lib/once.c           | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/once.h b/include/linux/once.h
index 9c98aaa87cbc..724724918e8b 100644
--- a/include/linux/once.h
+++ b/include/linux/once.h
@@ -5,7 +5,7 @@
 #include <linux/jump_label.h>
 
 bool __do_once_start(bool *done, unsigned long *flags);
-void __do_once_done(bool *done, struct static_key *once_key,
+void __do_once_done(bool *done, struct static_key_true *once_key,
 		    unsigned long *flags);
 
 /* Call a function exactly once. The idea of DO_ONCE() is to perform
@@ -38,8 +38,8 @@ void __do_once_done(bool *done, struct static_key *once_key,
 	({								     \
 		bool ___ret = false;					     \
 		static bool ___done = false;				     \
-		static struct static_key ___once_key = STATIC_KEY_INIT_TRUE; \
-		if (static_key_true(&___once_key)) {			     \
+		static DEFINE_STATIC_KEY_TRUE(___once_key);		     \
+		if (static_branch_unlikely(&___once_key)) {		     \
 			unsigned long ___flags;				     \
 			___ret = __do_once_start(&___done, &___flags);	     \
 			if (unlikely(___ret)) {				     \
diff --git a/lib/once.c b/lib/once.c
index 05c8604627eb..831c5a6b0bb2 100644
--- a/lib/once.c
+++ b/lib/once.c
@@ -5,7 +5,7 @@
 
 struct once_work {
 	struct work_struct work;
-	struct static_key *key;
+	struct static_key_true *key;
 };
 
 static void once_deferred(struct work_struct *w)
@@ -14,11 +14,11 @@ static void once_deferred(struct work_struct *w)
 
 	work = container_of(w, struct once_work, work);
 	BUG_ON(!static_key_enabled(work->key));
-	static_key_slow_dec(work->key);
+	static_branch_disable(work->key);
 	kfree(work);
 }
 
-static void once_disable_jump(struct static_key *key)
+static void once_disable_jump(struct static_key_true *key)
 {
 	struct once_work *w;
 
@@ -51,7 +51,7 @@ bool __do_once_start(bool *done, unsigned long *flags)
 }
 EXPORT_SYMBOL(__do_once_start);
 
-void __do_once_done(bool *done, struct static_key *once_key,
+void __do_once_done(bool *done, struct static_key_true *once_key,
 		    unsigned long *flags)
 	__releases(once_lock)
 {
-- 
2.14.2.920.gcf0c67979c-goog

^ permalink raw reply related

* Re: [PATCH net] net: enable interface alias removal via rtnl
From: David Ahern @ 2017-10-09 21:17 UTC (permalink / raw)
  To: nicolas.dichtel, Oliver Hartkopp, davem
  Cc: netdev, Oliver Hartkopp, Stephen Hemminger
In-Reply-To: <3e705d90-9b9a-aa02-e1d9-e3a944953014@6wind.com>

On 10/9/17 9:25 AM, Nicolas Dichtel wrote:
> Le 09/10/2017 à 16:02, David Ahern a écrit :
>> On 10/9/17 2:23 AM, Nicolas Dichtel wrote:
>>> Le 06/10/2017 à 22:10, Oliver Hartkopp a écrit :
>>>>
>>>>
>>>> On 10/06/2017 08:18 PM, David Ahern wrote:
>>>>> On 10/5/17 4:19 AM, Nicolas Dichtel wrote:
>>>>>> IFLA_IFALIAS is defined as NLA_STRING. It means that the minimal length of
>>>>>> the attribute is 1 ("\0"). However, to remove an alias, the attribute
>>>>>> length must be 0 (see dev_set_alias()).
>>>>>
>>>>> why not add a check in dev_set_alias that if len is 1 and the 1
>>>>> character is '\0' it means remove the alias?
>>> Because it requires an iproute2 patch. iproute2 doesn't send the '\0'. With the
>>> command 'ip link set dummy0 alias ""', the attribute length is 0.
>>
>> iproute2 needs the feature for 0-len strings or perhaps a 'noalias' option.
> iproute2 needs nothing ...
> 
>>
>> You can reset the alias using the sysfs file. Given that there is a
>> workaround for existing kernels and userspace, upstream can get fixed
>> without changing the UAPI.
>>
> I don't get the point with the UAPI. What will be broken?

never mind; I see the error of my ways.

> I don't see why allowing an attribute with no data is a problem.
> 

I remember the problem now. I made a patch back in March 2016 that
adjusted the policy validation to allow 0-length string. I never sent it
and forgot about it until today. You changing ifla_policy to NLA_BINARY
is achieving the same thing.

I think a comment above the policy line is warranted that clarifies
IFLA_IFALIAS is a string but to allow a 0-length string to remove the
alias NLA_BINARY is used for policy validation.

Comparing the validation done for NLA_STRING vs NLA_BINARY it does
change the behavior for 256-character strings.

^ permalink raw reply

* Re: [PATCH] once: switch to new jump label API
From: David Miller @ 2017-10-09 21:17 UTC (permalink / raw)
  To: daniel; +Cc: ebiggers3, hannes, netdev, linux-kernel, jbaron, peterz, ebiggers
In-Reply-To: <59DBE6C2.40104@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 09 Oct 2017 23:14:42 +0200

> Given original code was accepted against net-next tree as major
> users of the api are networking related anyway, it should be fine
> here as well to route through this tree. Maybe resend the patch with
> a [PATCH net-next] in the subject line (as usually done) to make the
> targeted tree more clear.

Indeed, please do, sorry for not replying.

^ permalink raw reply

* Re: [PATCH] once: switch to new jump label API
From: Daniel Borkmann @ 2017-10-09 21:14 UTC (permalink / raw)
  To: Eric Biggers, Hannes Frederic Sowa
  Cc: David S. Miller, netdev, linux-kernel, Jason Baron,
	Peter Zijlstra, Eric Biggers
In-Reply-To: <20171009202708.GB67463@gmail.com>

On 10/09/2017 10:27 PM, Eric Biggers wrote:
> On Fri, Sep 15, 2017 at 09:07:51PM -0700, Eric Biggers wrote:
>> On Tue, Aug 22, 2017 at 02:44:41PM -0400, Hannes Frederic Sowa wrote:
>>> Eric Biggers <ebiggers3@gmail.com> writes:
>>>> From: Eric Biggers <ebiggers@google.com>
>>>>
>>>> Switch the DO_ONCE() macro from the deprecated jump label API to the new
>>>> one.  The new one is more readable, and for DO_ONCE() it also makes the
>>>> generated code more icache-friendly: now the one-time initialization
>>>> code is placed out-of-line at the jump target, rather than at the inline
>>>> fallthrough case.
>>>>
>>>> Signed-off-by: Eric Biggers <ebiggers@google.com>
>>>
>>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org.
>>>
>>> Thanks!
>>
>> Great!  Who though is the maintainer for this code?  It seems it was originally
>> taken by David Miller through the networking tree.  David, are you taking
>> further patches to the "once" functions, or should I be trying to get this into
>> -mm, or somewhere else?
>>
>> Eric
>
> Ping.

Given original code was accepted against net-next tree as major users of
the api are networking related anyway, it should be fine here as well to
route through this tree. Maybe resend the patch with a [PATCH net-next]
in the subject line (as usually done) to make the targeted tree more clear.

^ permalink raw reply

* Re: [PATCH v6 05/11] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Maxime Ripard @ 2017-10-09 21:08 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: Andrew Lunn, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20171008183340.GA21531@Red>

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

On Sun, Oct 08, 2017 at 06:33:40PM +0000, Corentin Labbe wrote:
> On Thu, Sep 28, 2017 at 09:37:08AM +0200, Corentin Labbe wrote:
> > On Wed, Sep 27, 2017 at 04:02:10PM +0200, Andrew Lunn wrote:
> > > Hi Corentin
> > > 
> > > > +Required properties for the mdio-mux node:
> > > > +  - compatible = "mdio-mux"
> > > 
> > > This is too generic. Please add a more specific compatible for this
> > > particular mux. You can keep "mdio-mux", since that is what the MDIO
> > > subsystem will look for.
> > > 
> > 
> > I will add allwinner,sun8i-h3-mdio-mux
> > 
> > > > +Required properties of the integrated phy node:
> > > >  - clocks: a phandle to the reference clock for the EPHY
> > > >  - resets: a phandle to the reset control for the EPHY
> > > > +- phy-is-integrated
> > > 
> > > So the last thing you said is that the mux is not the problem
> > > here. Something else is locking up. Did you discover what?
> > > 
> > > I really would like phy-is-integrated to go away.
> > > 
> > 
> > I have found the problem: by enabling ephy clk/reset the timeout does not occur anymore.
> > So we could remove phy-is-integrated by:
> > Moving internal phy clk/reset handling in mdio_mux_syscon_switch_fn()
> > But this means:
> > - getting internalphy node always by manually get internal_mdio/internal_phy (and not by the given phyhandle)
> > - doing some unnecessary tasks (enable/scan/disable) when external_phy is needed
> > 
> > Regards
> 
> Hello all
> 
> Below is the current patch, as you can read, it does not use anymore the phy-is-integrated property.
> So now, the mdio-mux must always enable the internal mdio when switch_fn ask for it and so reset MAC and so need to enable ephy clk/reset.
> But for this I need a reference to thoses clock and reset. (this is done in get_ephy_nodes)
> The current version set those clock in mdio-mux node, and as you can see it is already ugly (lots of get next node),
> if the clk/rst nodes were as it should be, in phy nodes, it will be more bad.
> 
> So, since the MAC have a dependency on thoses clk/rst nodes for
> doing reset(), I seek a proper way to get references on it.
>
> OR do you agree that putting ephy clk/rst in emac is acceptable ?

Why not just parsing the DT child nodes looking for resets and clocks
properties? The usual PHY don't have that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [net-next V5 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: Daniel Borkmann @ 2017-10-09 20:56 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, jakub.kicinski, Michael S. Tsirkin, pavel.odintsov,
	Jason Wang, mchan, John Fastabend, peter.waskiewicz.jr,
	Daniel Borkmann, Alexei Starovoitov, Andy Gospodarek
In-Reply-To: <20171009195924.662f1586@redhat.com>

On 10/09/2017 07:59 PM, Jesper Dangaard Brouer wrote:
[...]
>>> +static void *cpu_map_lookup_elem(struct bpf_map *map, void *key)
>>> +{
>>> +	struct bpf_cpu_map_entry *rcpu =
>>> +		__cpu_map_lookup_elem(map, *(u32 *)key);
>>> +
>>> +	return rcpu ? &rcpu->qsize : NULL;
>>
>> I still think from my prior email/comment that we should use per-cpu
>> scratch buffer here. Would be nice to keep the guarantee that noone
>> can modify it, it's just a tiny change.
>
> Well, it's no-longer really needed, right(?), as this patchset update,
> change that bpf-side cannot invoke this.  The userspace-side reading
> this will get a copy.

Ah sorry, you're right, the related change happens in later patch, I
missed that; would be good to avoid a split in future or other option
is to forbid usage initially in check_map_func_compatibility() by
bailing out in BPF_MAP_TYPE_CPUMAP case unconditionally, and then
enabling it for BPF_FUNC_redirect_map in next step, such that should
someone accidentally only backport this patch, we don't allow for
unintended misuse.

Thanks,
Daniel

^ permalink raw reply

* [PATCH 3/3] net/atm: Adjust 121 checks for null pointers
From: SF Markus Elfring @ 2017-10-09 20:52 UTC (permalink / raw)
  To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
	Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
	Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
	Mitchell Blank Jr, Roopa Prabhu
  Cc: LKML, kernel-janitors
In-Reply-To: <10a92558-1105-b947-86a2-6ac763cca36d@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 22:22:45 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/br2684.c      |  17 ++++----
 net/atm/clip.c        |   2 +-
 net/atm/lec.c         |  38 ++++++++---------
 net/atm/mpc.c         | 114 +++++++++++++++++++++++++-------------------------
 net/atm/mpoa_caches.c |  63 ++++++++++++++--------------
 net/atm/pppoatm.c     |  11 +++--
 net/atm/proc.c        |   2 +-
 7 files changed, 123 insertions(+), 124 deletions(-)

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index f5b601c01f38..d3f8ec90556a 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -212,7 +212,7 @@ static int br2684_xmit_vcc(struct sk_buff *skb, struct net_device *dev,
 		struct sk_buff *skb2 = skb_realloc_headroom(skb, minheadroom);
 		brvcc->copies_needed++;
 		dev_kfree_skb(skb);
-		if (skb2 == NULL) {
+		if (!skb2) {
 			brvcc->copies_failed++;
 			return 0;
 		}
@@ -299,7 +299,7 @@ static netdev_tx_t br2684_start_xmit(struct sk_buff *skb,
 	pr_debug("skb_dst(skb)=%p\n", skb_dst(skb));
 	read_lock(&devs_lock);
 	brvcc = pick_outgoing_vcc(skb, brdev);
-	if (brvcc == NULL) {
+	if (!brvcc) {
 		pr_debug("no vcc attached to dev %s\n", dev->name);
 		dev->stats.tx_errors++;
 		dev->stats.tx_carrier_errors++;
@@ -372,13 +372,13 @@ static int br2684_setfilt(struct atm_vcc *atmvcc, void __user * arg)
 		struct br2684_dev *brdev;
 		read_lock(&devs_lock);
 		brdev = BRPRIV(br2684_find_dev(&fs.ifspec));
-		if (brdev == NULL || list_empty(&brdev->brvccs) ||
+		if (!brdev || list_empty(&brdev->brvccs) ||
 		    brdev->brvccs.next != brdev->brvccs.prev)	/* >1 VCC */
 			brvcc = NULL;
 		else
 			brvcc = list_entry_brvcc(brdev->brvccs.next);
 		read_unlock(&devs_lock);
-		if (brvcc == NULL)
+		if (!brvcc)
 			return -ESRCH;
 	} else
 		brvcc = BR2684_VCC(atmvcc);
@@ -427,8 +427,7 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 	struct br2684_dev *brdev = BRPRIV(net_dev);
 
 	pr_debug("\n");
-
-	if (unlikely(skb == NULL)) {
+	if (unlikely(!skb)) {
 		/* skb==NULL means VCC is being destroyed */
 		br2684_close_vcc(brvcc);
 		if (list_empty(&brdev->brvccs)) {
@@ -550,13 +549,13 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 	atomic_set(&brvcc->qspace, 2);
 	write_lock_irq(&devs_lock);
 	net_dev = br2684_find_dev(&be.ifspec);
-	if (net_dev == NULL) {
+	if (!net_dev) {
 		pr_err("tried to attach to non-existent device\n");
 		err = -ENXIO;
 		goto error;
 	}
 	brdev = BRPRIV(net_dev);
-	if (atmvcc->push == NULL) {
+	if (!atmvcc->push) {
 		err = -EBADFD;
 		goto error;
 	}
@@ -839,7 +838,7 @@ static int __init br2684_init(void)
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry *p;
 	p = proc_create("br2684", 0, atm_proc_root, &br2684_proc_ops);
-	if (p == NULL)
+	if (!p)
 		return -ENOMEM;
 #endif
 	register_atm_ioctl(&br2684_ioctl_ops);
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 041d519b8771..b45bfcb6cc1b 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -809,7 +809,7 @@ static void *clip_seq_vcc_walk(struct clip_seq_state *state,
 	struct clip_vcc *vcc = state->vcc;
 
 	vcc = clip_seq_next_vcc(e, vcc);
-	if (vcc && pos != NULL) {
+	if (vcc && pos) {
 		while (*pos) {
 			vcc = clip_seq_next_vcc(e, vcc);
 			if (!vcc)
diff --git a/net/atm/lec.c b/net/atm/lec.c
index 74a794602412..4f94c6ed893c 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -139,7 +139,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
 		struct atmlec_msg *mesg;
 
 		skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-		if (skb2 == NULL)
+		if (!skb2)
 			return;
 		skb2->len = sizeof(struct atmlec_msg);
 		mesg = (struct atmlec_msg *)skb2->data;
@@ -264,7 +264,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
 					       min_frame_size - skb->truesize,
 					       GFP_ATOMIC);
 			dev_kfree_skb(skb);
-			if (skb2 == NULL) {
+			if (!skb2) {
 				dev->stats.tx_dropped++;
 				return NETDEV_TX_OK;
 			}
@@ -431,7 +431,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 		pr_debug("%s: bridge zeppelin asks about %pM\n",
 			 dev->name, mesg->content.proxy.mac_addr);
 
-		if (br_fdb_test_addr_hook == NULL)
+		if (!br_fdb_test_addr_hook)
 			break;
 
 		if (br_fdb_test_addr_hook(dev, mesg->content.proxy.mac_addr)) {
@@ -442,7 +442,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 			pr_debug("%s: entry found, responding to zeppelin\n",
 				 dev->name);
 			skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-			if (skb2 == NULL)
+			if (!skb2)
 				break;
 			skb2->len = sizeof(struct atmlec_msg);
 			skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
@@ -520,7 +520,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
 	mesg = (struct atmlec_msg *)skb->data;
 	memset(mesg, 0, sizeof(struct atmlec_msg));
 	mesg->type = type;
-	if (data != NULL)
+	if (data)
 		mesg->sizeoftlvs = data->len;
 	if (mac_addr)
 		ether_addr_copy(mesg->content.normal.mac_addr, mac_addr);
@@ -534,7 +534,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk);
 
-	if (data != NULL) {
+	if (data) {
 		pr_debug("about to send %d bytes of data\n", data->len);
 		atm_force_charge(priv->lecd, data->truesize);
 		skb_queue_tail(&sk->sk_receive_queue, data);
@@ -663,7 +663,7 @@ static void lec_pop(struct atm_vcc *vcc, struct sk_buff *skb)
 	struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
 	struct net_device *dev = skb->dev;
 
-	if (vpriv == NULL) {
+	if (!vpriv) {
 		pr_info("vpriv = NULL!?!?!?\n");
 		return;
 	}
@@ -1066,7 +1066,7 @@ static void __exit lane_module_cleanup(void)
 	deregister_atm_ioctl(&lane_ioctl_ops);
 
 	for (i = 0; i < MAX_LEC_ITF; i++) {
-		if (dev_lec[i] != NULL) {
+		if (dev_lec[i]) {
 			unregister_netdev(dev_lec[i]);
 			free_netdev(dev_lec[i]);
 			dev_lec[i] = NULL;
@@ -1097,11 +1097,11 @@ static int lane2_resolve(struct net_device *dev, const u8 *dst_mac, int force,
 		spin_lock_irqsave(&priv->lec_arp_lock, flags);
 		table = lec_arp_find(priv, dst_mac);
 		spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
-		if (table == NULL)
+		if (!table)
 			return -1;
 
 		*tlvs = kmemdup(table->tlvs, table->sizeoftlvs, GFP_ATOMIC);
-		if (*tlvs == NULL)
+		if (!*tlvs)
 			return -1;
 
 		*sizeoftlvs = table->sizeoftlvs;
@@ -1109,12 +1109,12 @@ static int lane2_resolve(struct net_device *dev, const u8 *dst_mac, int force,
 		return 0;
 	}
 
-	if (sizeoftlvs == NULL)
+	if (!sizeoftlvs)
 		retval = send_to_lecd(priv, l_arp_xmt, dst_mac, NULL, NULL);
 
 	else {
 		skb = alloc_skb(*sizeoftlvs, GFP_ATOMIC);
-		if (skb == NULL)
+		if (!skb)
 			return -1;
 		skb->len = *sizeoftlvs;
 		skb_copy_to_linear_data(skb, *tlvs, *sizeoftlvs);
@@ -1143,12 +1143,12 @@ static int lane2_associate_req(struct net_device *dev, const u8 *lan_dst,
 	kfree(priv->tlvs);	/* NULL if there was no previous association */
 
 	priv->tlvs = kmemdup(tlvs, sizeoftlvs, GFP_KERNEL);
-	if (priv->tlvs == NULL)
+	if (!priv->tlvs)
 		return 0;
 	priv->sizeoftlvs = sizeoftlvs;
 
 	skb = alloc_skb(sizeoftlvs, GFP_ATOMIC);
-	if (skb == NULL)
+	if (!skb)
 		return 0;
 	skb->len = sizeoftlvs;
 	skb_copy_to_linear_data(skb, tlvs, sizeoftlvs);
@@ -1181,13 +1181,13 @@ static void lane2_associate_ind(struct net_device *dev, const u8 *mac_addr,
 				 */
 	struct lec_arp_table *entry = lec_arp_find(priv, mac_addr);
 
-	if (entry == NULL)
+	if (!entry)
 		return;		/* should not happen */
 
 	kfree(entry->tlvs);
 
 	entry->tlvs = kmemdup(tlvs, sizeoftlvs, GFP_KERNEL);
-	if (entry->tlvs == NULL)
+	if (!entry->tlvs)
 		return;
 	entry->sizeoftlvs = sizeoftlvs;
 #endif
@@ -1854,7 +1854,7 @@ lec_arp_update(struct lec_priv *priv, const unsigned char *mac_addr,
 
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
 	entry = lec_arp_find(priv, mac_addr);
-	if (entry == NULL && targetless_le_arp)
+	if (!entry && targetless_le_arp)
 		goto out;	/*
 				 * LANE2: ignore targetless LE_ARPs for which
 				 * we have no entry in the cache. 7.1.30
@@ -1965,7 +1965,7 @@ lec_vcc_added(struct lec_priv *priv, const struct atmlec_ioc *ioc_data,
 		entry->old_recv_push = old_push;
 #endif
 		entry = make_entry(priv, bus_mac);
-		if (entry == NULL)
+		if (!entry)
 			goto out;
 		del_timer(&entry->timer);
 		memcpy(entry->atm_addr, ioc_data->atm_addr, ATM_ESA_LEN);
@@ -1990,7 +1990,7 @@ lec_vcc_added(struct lec_priv *priv, const struct atmlec_ioc *ioc_data,
 			 ioc_data->atm_addr[16], ioc_data->atm_addr[17],
 			 ioc_data->atm_addr[18], ioc_data->atm_addr[19]);
 		entry = make_entry(priv, bus_mac);
-		if (entry == NULL)
+		if (!entry)
 			goto out;
 		memcpy(entry->atm_addr, ioc_data->atm_addr, ATM_ESA_LEN);
 		eth_zero_addr(entry->mac_addr);
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index d6729d797107..ece715f8c9f7 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -129,7 +129,7 @@ static struct mpoa_client *find_mpc_by_itfnum(int itf)
 	struct mpoa_client *mpc;
 
 	mpc = mpcs;  /* our global linked list */
-	while (mpc != NULL) {
+	while (mpc) {
 		if (mpc->dev_num == itf)
 			return mpc;
 		mpc = mpc->next;
@@ -143,7 +143,7 @@ static struct mpoa_client *find_mpc_by_vcc(struct atm_vcc *vcc)
 	struct mpoa_client *mpc;
 
 	mpc = mpcs;  /* our global linked list */
-	while (mpc != NULL) {
+	while (mpc) {
 		if (mpc->mpoad_vcc == vcc)
 			return mpc;
 		mpc = mpc->next;
@@ -157,7 +157,7 @@ static struct mpoa_client *find_mpc_by_lec(struct net_device *dev)
 	struct mpoa_client *mpc;
 
 	mpc = mpcs;  /* our global linked list */
-	while (mpc != NULL) {
+	while (mpc) {
 		if (mpc->dev == dev)
 			return mpc;
 		mpc = mpc->next;
@@ -178,7 +178,7 @@ struct atm_mpoa_qos *atm_mpoa_add_qos(__be32 dst_ip, struct atm_qos *qos)
 	struct atm_mpoa_qos *entry;
 
 	entry = atm_mpoa_search_qos(dst_ip);
-	if (entry != NULL) {
+	if (entry) {
 		entry->qos = *qos;
 		return entry;
 	}
@@ -217,7 +217,7 @@ int atm_mpoa_delete_qos(struct atm_mpoa_qos *entry)
 {
 	struct atm_mpoa_qos *curr;
 
-	if (entry == NULL)
+	if (!entry)
 		return 0;
 	if (entry == qos_head) {
 		qos_head = qos_head->next;
@@ -226,7 +226,7 @@ int atm_mpoa_delete_qos(struct atm_mpoa_qos *entry)
 	}
 
 	curr = qos_head;
-	while (curr != NULL) {
+	while (curr) {
 		if (curr->next == entry) {
 			curr->next = entry->next;
 			kfree(entry);
@@ -247,7 +247,7 @@ void atm_mpoa_disp_qos(struct seq_file *m)
 	seq_printf(m, "QoS entries for shortcuts:\n");
 	seq_printf(m, "IP address\n  TX:max_pcr pcr     min_pcr max_cdv max_sdu\n  RX:max_pcr pcr     min_pcr max_cdv max_sdu\n");
 
-	while (qos != NULL) {
+	while (qos) {
 		seq_printf(m, "%pI4\n     %-7d %-7d %-7d %-7d %-7d\n     %-7d %-7d %-7d %-7d %-7d\n",
 			   &qos->ipaddr,
 			   qos->qos.txtp.max_pcr,
@@ -280,7 +280,7 @@ static struct mpoa_client *alloc_mpc(void)
 	struct mpoa_client *mpc;
 
 	mpc = kzalloc(sizeof(*mpc), GFP_KERNEL);
-	if (mpc == NULL)
+	if (!mpc)
 		return NULL;
 	rwlock_init(&mpc->ingress_lock);
 	rwlock_init(&mpc->egress_lock);
@@ -381,7 +381,7 @@ static void lane2_assoc_ind(struct net_device *dev, const u8 *mac_addr,
 	dprintk("(%s) received TLV(s), ", dev->name);
 	dprintk("total length of all TLVs %d\n", sizeoftlvs);
 	mpc = find_mpc_by_lec(dev); /* Sampo-Fix: moved here from below */
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("(%s) no mpc\n", dev->name);
 		return;
 	}
@@ -445,7 +445,7 @@ static void lane2_assoc_ind(struct net_device *dev, const u8 *mac_addr,
 
 		tlvs = copy_macs(mpc, mac_addr, tlvs,
 				 number_of_mps_macs, mpoa_device_type);
-		if (tlvs == NULL)
+		if (!tlvs)
 			return;
 	}
 	if (end_of_tlvs - tlvs != 0)
@@ -507,9 +507,9 @@ static int send_via_shortcut(struct sk_buff *skb, struct mpoa_client *mpc)
 		 mpc->dev->name, ipaddr);
 
 	entry = mpc->in_ops->get(ipaddr, mpc);
-	if (entry == NULL) {
+	if (!entry) {
 		entry = mpc->in_ops->add_entry(ipaddr, mpc);
-		if (entry != NULL)
+		if (entry)
 			mpc->in_ops->put(entry);
 		return 1;
 	}
@@ -571,7 +571,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
 	int i = 0;
 
 	mpc = find_mpc_by_lec(dev); /* this should NEVER fail */
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("(%s) no MPC found\n", dev->name);
 		goto non_ip;
 	}
@@ -617,16 +617,16 @@ static int atm_mpoa_vcc_attach(struct atm_vcc *vcc, void __user *arg)
 		return -EINVAL;
 
 	mpc = find_mpc_by_itfnum(ioc_data.dev_num);
-	if (mpc == NULL)
+	if (!mpc)
 		return -EINVAL;
 
 	if (ioc_data.type == MPC_SOCKET_INGRESS) {
 		in_entry = mpc->in_ops->get(ipaddr, mpc);
-		if (in_entry == NULL ||
+		if (!in_entry ||
 		    in_entry->entry_state < INGRESS_RESOLVED) {
 			pr_info("(%s) did not find RESOLVED entry from ingress cache\n",
 				mpc->dev->name);
-			if (in_entry != NULL)
+			if (in_entry)
 				mpc->in_ops->put(in_entry);
 			return -EINVAL;
 		}
@@ -654,7 +654,7 @@ static void mpc_vcc_close(struct atm_vcc *vcc, struct net_device *dev)
 	eg_cache_entry *eg_entry;
 
 	mpc = find_mpc_by_lec(dev);
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("(%s) close for unknown MPC\n", dev->name);
 		return;
 	}
@@ -674,7 +674,7 @@ static void mpc_vcc_close(struct atm_vcc *vcc, struct net_device *dev)
 		mpc->eg_ops->put(eg_entry);
 	}
 
-	if (in_entry == NULL && eg_entry == NULL)
+	if (!in_entry && !eg_entry)
 		dprintk("(%s) unused vcc closed\n", dev->name);
 }
 
@@ -688,7 +688,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	char *tmp;
 
 	ddprintk("(%s)\n", dev->name);
-	if (skb == NULL) {
+	if (!skb) {
 		dprintk("(%s) null skb, closing VCC\n", dev->name);
 		mpc_vcc_close(vcc, dev);
 		return;
@@ -710,7 +710,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	atm_return(vcc, skb->truesize);
 
 	mpc = find_mpc_by_lec(dev);
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("(%s) unknown MPC\n", dev->name);
 		return;
 	}
@@ -735,7 +735,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	tag = *(__be32 *)tmp;
 
 	eg = mpc->eg_ops->get_by_tag(tag, mpc);
-	if (eg == NULL) {
+	if (!eg) {
 		pr_info("mpoa: (%s) Didn't find egress cache entry, tag = %u\n",
 			dev->name, tag);
 		purge_egress_shortcut(vcc, NULL);
@@ -747,7 +747,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	 * See if ingress MPC is using shortcut we opened as a return channel.
 	 * This means we have a bi-directional vcc opened by us.
 	 */
-	if (eg->shortcut == NULL) {
+	if (!eg->shortcut) {
 		eg->shortcut = vcc;
 		pr_info("(%s) egress SVC in use\n", dev->name);
 	}
@@ -757,7 +757,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	new_skb = skb_realloc_headroom(skb, eg->ctrl_info.DH_length);
 					/* LLC/SNAP is shorter than MAC header :( */
 	dev_kfree_skb_any(skb);
-	if (new_skb == NULL) {
+	if (!new_skb) {
 		mpc->eg_ops->put(eg);
 		return;
 	}
@@ -794,7 +794,7 @@ static int atm_mpoa_mpoad_attach(struct atm_vcc *vcc, int arg)
 	struct lec_priv *priv;
 	int err;
 
-	if (mpcs == NULL) {
+	if (!mpcs) {
 		init_timer(&mpc_timer);
 		mpc_timer_refresh();
 
@@ -807,10 +807,10 @@ static int atm_mpoa_mpoad_attach(struct atm_vcc *vcc, int arg)
 	}
 
 	mpc = find_mpc_by_itfnum(arg);
-	if (mpc == NULL) {
+	if (!mpc) {
 		dprintk("allocating new mpc for itf %d\n", arg);
 		mpc = alloc_mpc();
-		if (mpc == NULL)
+		if (!mpc)
 			return -ENOMEM;
 		mpc->dev_num = arg;
 		mpc->dev = find_lec_by_itfnum(arg);
@@ -869,7 +869,7 @@ static void mpoad_close(struct atm_vcc *vcc)
 	struct sk_buff *skb;
 
 	mpc = find_mpc_by_vcc(vcc);
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("did not find MPC\n");
 		return;
 	}
@@ -909,7 +909,7 @@ static int msg_from_mpoad(struct atm_vcc *vcc, struct sk_buff *skb)
 	struct k_message *mesg = (struct k_message *)skb->data;
 	WARN_ON(refcount_sub_and_test(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc));
 
-	if (mpc == NULL) {
+	if (!mpc) {
 		pr_info("no mpc found\n");
 		return 0;
 	}
@@ -974,13 +974,13 @@ int msg_to_mpoad(struct k_message *mesg, struct mpoa_client *mpc)
 	struct sk_buff *skb;
 	struct sock *sk;
 
-	if (mpc == NULL || !mpc->mpoad_vcc) {
+	if (!mpc || !mpc->mpoad_vcc) {
 		pr_info("mesg %d to a non-existent mpoad\n", mesg->type);
 		return -ENXIO;
 	}
 
 	skb = alloc_skb(sizeof(struct k_message), GFP_ATOMIC);
-	if (skb == NULL)
+	if (!skb)
 		return -ENOMEM;
 	skb_put(skb, sizeof(struct k_message));
 	skb_copy_to_linear_data(skb, mesg, sizeof(*mesg));
@@ -1013,10 +1013,10 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
 			break;
 		priv->lane2_ops->associate_indicator = lane2_assoc_ind;
 		mpc = find_mpc_by_itfnum(priv->itfnum);
-		if (mpc == NULL) {
+		if (!mpc) {
 			dprintk("allocating new mpc for %s\n", dev->name);
 			mpc = alloc_mpc();
-			if (mpc == NULL) {
+			if (!mpc) {
 				pr_info("no new mpc");
 				break;
 			}
@@ -1029,7 +1029,7 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
 	case NETDEV_UNREGISTER:
 		/* the lec device was deallocated */
 		mpc = find_mpc_by_lec(dev);
-		if (mpc == NULL)
+		if (!mpc)
 			break;
 		dprintk("device (%s) was deallocated\n", dev->name);
 		stop_mpc(mpc);
@@ -1039,9 +1039,9 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
 	case NETDEV_UP:
 		/* the dev was ifconfig'ed up */
 		mpc = find_mpc_by_lec(dev);
-		if (mpc == NULL)
+		if (!mpc)
 			break;
-		if (mpc->mpoad_vcc != NULL)
+		if (mpc->mpoad_vcc)
 			start_mpc(mpc, dev);
 		break;
 	case NETDEV_DOWN:
@@ -1050,9 +1050,9 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
 		 * upper layer stops
 		 */
 		mpc = find_mpc_by_lec(dev);
-		if (mpc == NULL)
+		if (!mpc)
 			break;
-		if (mpc->mpoad_vcc != NULL)
+		if (mpc->mpoad_vcc)
 			stop_mpc(mpc);
 		break;
 	case NETDEV_REBOOT:
@@ -1080,7 +1080,7 @@ static void MPOA_trigger_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 	in_cache_entry *entry;
 
 	entry = mpc->in_ops->get(dst_ip, mpc);
-	if (entry == NULL) {
+	if (!entry) {
 		entry = mpc->in_ops->add_entry(dst_ip, mpc);
 		entry->entry_state = INGRESS_RESOLVING;
 		msg->type = SND_MPOA_RES_RQST;
@@ -1134,7 +1134,7 @@ static void check_qos_and_open_shortcut(struct k_message *msg,
 			return;
 		}
 	}
-	if (eg_entry != NULL)
+	if (eg_entry)
 		client->eg_ops->put(eg_entry);
 
 	/* No luck in the egress cache we must open an ingress SVC */
@@ -1158,7 +1158,7 @@ static void MPOA_res_reply_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 		mpc->dev->name, &dst_ip);
 	ddprintk("(%s) entry = %p",
 		 mpc->dev->name, entry);
-	if (entry == NULL) {
+	if (!entry) {
 		pr_info("(%s) ARGH, received res. reply for an entry that doesn't exist.\n",
 			mpc->dev->name);
 		return;
@@ -1178,13 +1178,13 @@ static void MPOA_res_reply_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 	ddprintk_cont("entry->shortcut = %p\n", entry->shortcut);
 
 	if (entry->entry_state == INGRESS_RESOLVING &&
-	    entry->shortcut != NULL) {
+	    entry->shortcut) {
 		entry->entry_state = INGRESS_RESOLVED;
 		mpc->in_ops->put(entry);
 		return; /* Shortcut already open... */
 	}
 
-	if (entry->shortcut != NULL) {
+	if (entry->shortcut) {
 		pr_info("(%s) entry->shortcut != NULL, impossible!\n",
 			mpc->dev->name);
 		mpc->in_ops->put(entry);
@@ -1205,7 +1205,7 @@ static void ingress_purge_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 	__be32 mask = msg->ip_mask;
 	in_cache_entry *entry = mpc->in_ops->get_with_mask(dst_ip, mpc, mask);
 
-	if (entry == NULL) {
+	if (!entry) {
 		pr_info("(%s) purge for a non-existing entry, ip = %pI4\n",
 			mpc->dev->name, &dst_ip);
 		return;
@@ -1219,7 +1219,7 @@ static void ingress_purge_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 		write_unlock_bh(&mpc->ingress_lock);
 		mpc->in_ops->put(entry);
 		entry = mpc->in_ops->get_with_mask(dst_ip, mpc, mask);
-	} while (entry != NULL);
+	} while (entry);
 }
 
 static void egress_purge_rcvd(struct k_message *msg, struct mpoa_client *mpc)
@@ -1227,7 +1227,7 @@ static void egress_purge_rcvd(struct k_message *msg, struct mpoa_client *mpc)
 	__be32 cache_id = msg->content.eg_info.cache_id;
 	eg_cache_entry *entry = mpc->eg_ops->get_by_cache_id(cache_id, mpc);
 
-	if (entry == NULL) {
+	if (!entry) {
 		dprintk("(%s) purge for a non-existing entry\n",
 			mpc->dev->name);
 		return;
@@ -1247,13 +1247,13 @@ static void purge_egress_shortcut(struct atm_vcc *vcc, eg_cache_entry *entry)
 	struct sk_buff *skb;
 
 	dprintk("entering\n");
-	if (vcc == NULL) {
+	if (!vcc) {
 		pr_info("vcc == NULL\n");
 		return;
 	}
 
 	skb = alloc_skb(sizeof(struct k_message), GFP_ATOMIC);
-	if (skb == NULL) {
+	if (!skb) {
 		pr_info("out of memory\n");
 		return;
 	}
@@ -1262,7 +1262,7 @@ static void purge_egress_shortcut(struct atm_vcc *vcc, eg_cache_entry *entry)
 	memset(skb->data, 0, sizeof(struct k_message));
 	purge_msg = (struct k_message *)skb->data;
 	purge_msg->type = DATA_PLANE_PURGE;
-	if (entry != NULL)
+	if (entry)
 		purge_msg->content.eg_info = entry->ctrl_info;
 
 	atm_force_charge(vcc, skb->truesize);
@@ -1291,7 +1291,7 @@ static void mps_death(struct k_message *msg, struct mpoa_client *mpc)
 	/* FIXME: This knows too much of the cache structure */
 	read_lock_irq(&mpc->egress_lock);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		purge_egress_shortcut(entry->shortcut, entry);
 		entry = entry->next;
 	}
@@ -1310,7 +1310,7 @@ static void MPOA_cache_impos_rcvd(struct k_message *msg,
 	holding_time = msg->content.eg_info.holding_time;
 	dprintk("(%s) entry = %p, holding_time = %u\n",
 		mpc->dev->name, entry, holding_time);
-	if (entry == NULL && holding_time) {
+	if (!entry && holding_time) {
 		entry = mpc->eg_ops->add_entry(msg, mpc);
 		mpc->eg_ops->put(entry);
 		return;
@@ -1372,7 +1372,7 @@ static void set_mps_mac_addr_rcvd(struct k_message *msg,
 		kfree(client->mps_macs);
 	client->number_of_mps_macs = 0;
 	client->mps_macs = kmemdup(msg->MPS_ctrl, ETH_ALEN, GFP_KERNEL);
-	if (client->mps_macs == NULL) {
+	if (!client->mps_macs) {
 		pr_info("out of memory\n");
 		return;
 	}
@@ -1392,7 +1392,7 @@ static void clean_up(struct k_message *msg, struct mpoa_client *mpc, int action)
 	/* FIXME: This knows too much of the cache structure */
 	read_lock_irq(&mpc->egress_lock);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		msg->content.eg_info = entry->ctrl_info;
 		dprintk("cache_id %u\n", entry->ctrl_info.cache_id);
 		msg_to_mpoad(msg, mpc);
@@ -1418,7 +1418,7 @@ static void mpc_cache_check(unsigned long checking_time)
 	static unsigned long previous_resolving_check_time;
 	static unsigned long previous_refresh_time;
 
-	while (mpc != NULL) {
+	while (mpc) {
 		mpc->in_ops->clear_count(mpc);
 		mpc->eg_ops->clear_expired(mpc);
 		if (checking_time - previous_resolving_check_time >
@@ -1494,12 +1494,12 @@ static void __exit atm_mpoa_cleanup(void)
 
 	mpc = mpcs;
 	mpcs = NULL;
-	while (mpc != NULL) {
+	while (mpc) {
 		tmp = mpc->next;
-		if (mpc->dev != NULL) {
+		if (mpc->dev) {
 			stop_mpc(mpc);
 			priv = netdev_priv(mpc->dev);
-			if (priv->lane2_ops != NULL)
+			if (priv->lane2_ops)
 				priv->lane2_ops->associate_indicator = NULL;
 		}
 		ddprintk("about to clear caches\n");
@@ -1516,7 +1516,7 @@ static void __exit atm_mpoa_cleanup(void)
 
 	qos = qos_head;
 	qos_head = NULL;
-	while (qos != NULL) {
+	while (qos) {
 		nextqos = qos->next;
 		dprintk("freeing qos entry %p\n", qos);
 		kfree(qos);
diff --git a/net/atm/mpoa_caches.c b/net/atm/mpoa_caches.c
index 23f36e5a20ee..c147fe916446 100644
--- a/net/atm/mpoa_caches.c
+++ b/net/atm/mpoa_caches.c
@@ -38,7 +38,7 @@ static in_cache_entry *in_cache_get(__be32 dst_ip,
 
 	read_lock_bh(&client->ingress_lock);
 	entry = client->in_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->ctrl_info.in_dst_ip == dst_ip) {
 			refcount_inc(&entry->use);
 			read_unlock_bh(&client->ingress_lock);
@@ -59,7 +59,7 @@ static in_cache_entry *in_cache_get_with_mask(__be32 dst_ip,
 
 	read_lock_bh(&client->ingress_lock);
 	entry = client->in_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if ((entry->ctrl_info.in_dst_ip & mask) == (dst_ip & mask)) {
 			refcount_inc(&entry->use);
 			read_unlock_bh(&client->ingress_lock);
@@ -80,7 +80,7 @@ static in_cache_entry *in_cache_get_by_vcc(struct atm_vcc *vcc,
 
 	read_lock_bh(&client->ingress_lock);
 	entry = client->in_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->shortcut == vcc) {
 			refcount_inc(&entry->use);
 			read_unlock_bh(&client->ingress_lock);
@@ -108,7 +108,7 @@ static in_cache_entry *in_cache_add_entry(__be32 dst_ip,
 	write_lock_bh(&client->ingress_lock);
 	entry->next = client->in_cache;
 	entry->prev = NULL;
-	if (client->in_cache != NULL)
+	if (client->in_cache)
 		client->in_cache->prev = entry;
 	client->in_cache = entry;
 
@@ -133,7 +133,7 @@ static int cache_hit(in_cache_entry *entry, struct mpoa_client *mpc)
 	struct k_message msg;
 
 	entry->count++;
-	if (entry->entry_state == INGRESS_RESOLVED && entry->shortcut != NULL)
+	if (entry->entry_state == INGRESS_RESOLVED && entry->shortcut)
 		return OPEN;
 
 	if (entry->entry_state == INGRESS_REFRESHING) {
@@ -142,18 +142,18 @@ static int cache_hit(in_cache_entry *entry, struct mpoa_client *mpc)
 			msg.content.in_info = entry->ctrl_info;
 			memcpy(msg.MPS_ctrl, mpc->mps_ctrl_addr, ATM_ESA_LEN);
 			qos = atm_mpoa_search_qos(entry->ctrl_info.in_dst_ip);
-			if (qos != NULL)
+			if (qos)
 				msg.qos = qos->qos;
 			msg_to_mpoad(&msg, mpc);
 			do_gettimeofday(&(entry->reply_wait));
 			entry->entry_state = INGRESS_RESOLVING;
 		}
-		if (entry->shortcut != NULL)
+		if (entry->shortcut)
 			return OPEN;
 		return CLOSED;
 	}
 
-	if (entry->entry_state == INGRESS_RESOLVING && entry->shortcut != NULL)
+	if (entry->entry_state == INGRESS_RESOLVING && entry->shortcut)
 		return OPEN;
 
 	if (entry->count > mpc->parameters.mpc_p1 &&
@@ -165,7 +165,7 @@ static int cache_hit(in_cache_entry *entry, struct mpoa_client *mpc)
 		memcpy(msg.MPS_ctrl, mpc->mps_ctrl_addr, ATM_ESA_LEN);
 		msg.content.in_info = entry->ctrl_info;
 		qos = atm_mpoa_search_qos(entry->ctrl_info.in_dst_ip);
-		if (qos != NULL)
+		if (qos)
 			msg.qos = qos->qos;
 		msg_to_mpoad(&msg, mpc);
 		do_gettimeofday(&(entry->reply_wait));
@@ -195,23 +195,24 @@ static void in_cache_remove_entry(in_cache_entry *entry,
 	dprintk("removing an ingress entry, ip = %pI4\n",
 		&entry->ctrl_info.in_dst_ip);
 
-	if (entry->prev != NULL)
+	if (entry->prev)
 		entry->prev->next = entry->next;
 	else
 		client->in_cache = entry->next;
-	if (entry->next != NULL)
+	if (entry->next)
 		entry->next->prev = entry->prev;
 	client->in_ops->put(entry);
-	if (client->in_cache == NULL && client->eg_cache == NULL) {
+	if (!client->in_cache && !client->eg_cache) {
 		msg.type = STOP_KEEP_ALIVE_SM;
 		msg_to_mpoad(&msg, client);
 	}
 
 	/* Check if the egress side still uses this VCC */
-	if (vcc != NULL) {
+	if (vcc) {
 		eg_cache_entry *eg_entry = client->eg_ops->get_by_vcc(vcc,
 								      client);
-		if (eg_entry != NULL) {
+
+		if (eg_entry) {
 			client->eg_ops->put(eg_entry);
 			return;
 		}
@@ -230,7 +231,7 @@ static void clear_count_and_expired(struct mpoa_client *client)
 
 	write_lock_bh(&client->ingress_lock);
 	entry = client->in_cache;
-	while (entry != NULL) {
+	while (entry) {
 		entry->count = 0;
 		next_entry = entry->next;
 		if ((now.tv_sec - entry->tv.tv_sec)
@@ -257,7 +258,7 @@ static void check_resolving_entries(struct mpoa_client *client)
 
 	read_lock_bh(&client->ingress_lock);
 	entry = client->in_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->entry_state == INGRESS_RESOLVING) {
 			if ((now.tv_sec - entry->hold_down.tv_sec) <
 			    client->parameters.mpc_p6) {
@@ -283,7 +284,7 @@ static void check_resolving_entries(struct mpoa_client *client)
 				memcpy(msg.MPS_ctrl, client->mps_ctrl_addr, ATM_ESA_LEN);
 				msg.content.in_info = entry->ctrl_info;
 				qos = atm_mpoa_search_qos(entry->ctrl_info.in_dst_ip);
-				if (qos != NULL)
+				if (qos)
 					msg.qos = qos->qos;
 				msg_to_mpoad(&msg, client);
 				do_gettimeofday(&(entry->reply_wait));
@@ -304,7 +305,7 @@ static void refresh_entries(struct mpoa_client *client)
 	do_gettimeofday(&now);
 
 	read_lock_bh(&client->ingress_lock);
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->entry_state == INGRESS_RESOLVED) {
 			if (!(entry->refresh_time))
 				entry->refresh_time = (2 * (entry->ctrl_info.holding_time))/3;
@@ -323,7 +324,7 @@ static void refresh_entries(struct mpoa_client *client)
 static void in_destroy_cache(struct mpoa_client *mpc)
 {
 	write_lock_irq(&mpc->ingress_lock);
-	while (mpc->in_cache != NULL)
+	while (mpc->in_cache)
 		mpc->in_ops->remove_entry(mpc->in_cache, mpc);
 	write_unlock_irq(&mpc->ingress_lock);
 }
@@ -335,7 +336,7 @@ static eg_cache_entry *eg_cache_get_by_cache_id(__be32 cache_id,
 
 	read_lock_irq(&mpc->egress_lock);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->ctrl_info.cache_id == cache_id) {
 			refcount_inc(&entry->use);
 			read_unlock_irq(&mpc->egress_lock);
@@ -356,7 +357,7 @@ static eg_cache_entry *eg_cache_get_by_tag(__be32 tag, struct mpoa_client *mpc)
 
 	read_lock_irqsave(&mpc->egress_lock, flags);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->ctrl_info.tag == tag) {
 			refcount_inc(&entry->use);
 			read_unlock_irqrestore(&mpc->egress_lock, flags);
@@ -378,7 +379,7 @@ static eg_cache_entry *eg_cache_get_by_vcc(struct atm_vcc *vcc,
 
 	read_lock_irqsave(&mpc->egress_lock, flags);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->shortcut == vcc) {
 			refcount_inc(&entry->use);
 			read_unlock_irqrestore(&mpc->egress_lock, flags);
@@ -398,7 +399,7 @@ static eg_cache_entry *eg_cache_get_by_src_ip(__be32 ipaddr,
 
 	read_lock_irq(&mpc->egress_lock);
 	entry = mpc->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		if (entry->latest_ip_addr == ipaddr) {
 			refcount_inc(&entry->use);
 			read_unlock_irq(&mpc->egress_lock);
@@ -430,22 +431,22 @@ static void eg_cache_remove_entry(eg_cache_entry *entry,
 
 	vcc = entry->shortcut;
 	dprintk("removing an egress entry.\n");
-	if (entry->prev != NULL)
+	if (entry->prev)
 		entry->prev->next = entry->next;
 	else
 		client->eg_cache = entry->next;
-	if (entry->next != NULL)
+	if (entry->next)
 		entry->next->prev = entry->prev;
 	client->eg_ops->put(entry);
-	if (client->in_cache == NULL && client->eg_cache == NULL) {
+	if (!client->in_cache && !client->eg_cache) {
 		msg.type = STOP_KEEP_ALIVE_SM;
 		msg_to_mpoad(&msg, client);
 	}
 
 	/* Check if the ingress side still uses this VCC */
-	if (vcc != NULL) {
+	if (vcc) {
 		in_cache_entry *in_entry = client->in_ops->get_by_vcc(vcc, client);
-		if (in_entry != NULL) {
+		if (in_entry) {
 			client->in_ops->put(in_entry);
 			return;
 		}
@@ -469,7 +470,7 @@ static eg_cache_entry *eg_cache_add_entry(struct k_message *msg,
 	write_lock_irq(&client->egress_lock);
 	entry->next = client->eg_cache;
 	entry->prev = NULL;
-	if (client->eg_cache != NULL)
+	if (client->eg_cache)
 		client->eg_cache->prev = entry;
 	client->eg_cache = entry;
 
@@ -505,7 +506,7 @@ static void clear_expired(struct mpoa_client *client)
 
 	write_lock_irq(&client->egress_lock);
 	entry = client->eg_cache;
-	while (entry != NULL) {
+	while (entry) {
 		next_entry = entry->next;
 		if ((now.tv_sec - entry->tv.tv_sec)
 		   > entry->ctrl_info.holding_time) {
@@ -524,7 +525,7 @@ static void clear_expired(struct mpoa_client *client)
 static void eg_destroy_cache(struct mpoa_client *mpc)
 {
 	write_lock_irq(&mpc->egress_lock);
-	while (mpc->eg_cache != NULL)
+	while (mpc->eg_cache)
 		mpc->eg_ops->remove_entry(mpc->eg_cache, mpc);
 	write_unlock_irq(&mpc->egress_lock);
 }
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 21d9d341a619..890eb377e2dd 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -183,7 +183,7 @@ static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 {
 	struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
 	pr_debug("\n");
-	if (skb == NULL) {			/* VCC was closed */
+	if (!skb) {			/* VCC was closed */
 		struct module *module;
 
 		pr_debug("removing ATMPPP VCC %p\n", pvcc);
@@ -202,7 +202,7 @@ static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 		skb_pull(skb, LLC_LEN);
 		break;
 	case e_autodetect:
-		if (pvcc->chan.ppp == NULL) {	/* Not bound yet! */
+		if (!pvcc->chan.ppp) {	/* Not bound yet! */
 			kfree_skb(skb);
 			return;
 		}
@@ -324,14 +324,13 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
 		if (skb_headroom(skb) < LLC_LEN) {
 			struct sk_buff *n;
 			n = skb_realloc_headroom(skb, LLC_LEN);
-			if (n != NULL &&
-			    !pppoatm_may_send(pvcc, n->truesize)) {
+			if (n && !pppoatm_may_send(pvcc, n->truesize)) {
 				kfree_skb(n);
 				goto nospace;
 			}
 			consume_skb(skb);
 			skb = n;
-			if (skb == NULL) {
+			if (!skb) {
 				bh_unlock_sock(sk_atm(vcc));
 				return DROP_PACKET;
 			}
@@ -406,7 +405,7 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
 	    be.encaps != PPPOATM_ENCAPS_VC && be.encaps != PPPOATM_ENCAPS_LLC)
 		return -EINVAL;
 	pvcc = kzalloc(sizeof(*pvcc), GFP_KERNEL);
-	if (pvcc == NULL)
+	if (!pvcc)
 		return -ENOMEM;
 	pvcc->atmvcc = atmvcc;
 
diff --git a/net/atm/proc.c b/net/atm/proc.c
index 4caca2a90ec4..aa230079cf8a 100644
--- a/net/atm/proc.c
+++ b/net/atm/proc.c
@@ -118,7 +118,7 @@ static int __vcc_seq_open(struct inode *inode, struct file *file,
 	struct vcc_state *state;
 
 	state = __seq_open_private(file, ops, sizeof(*state));
-	if (state == NULL)
+	if (!state)
 		return -ENOMEM;
 
 	state->family = family;
-- 
2.14.2

^ permalink raw reply related

* [PATCH 2/3] net/atm: Improve a size determination in 12 functions
From: SF Markus Elfring @ 2017-10-09 20:50 UTC (permalink / raw)
  To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
	Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
	Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
	Mitchell Blank Jr, Roopa Prabhu
  Cc: LKML, kernel-janitors
In-Reply-To: <10a92558-1105-b947-86a2-6ac763cca36d@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 22:00:19 +0200

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/addr.c        | 2 +-
 net/atm/br2684.c      | 2 +-
 net/atm/clip.c        | 4 ++--
 net/atm/lec.c         | 6 +++---
 net/atm/mpc.c         | 4 ++--
 net/atm/mpoa_caches.c | 4 ++--
 net/atm/signaling.c   | 2 +-
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/atm/addr.c b/net/atm/addr.c
index dcda35c66f15..048e6d192818 100644
--- a/net/atm/addr.c
+++ b/net/atm/addr.c
@@ -86,7 +86,7 @@ int atm_add_addr(struct atm_dev *dev, const struct sockaddr_atmsvc *addr,
 			return -EEXIST;
 		}
 	}
-	this = kmalloc(sizeof(struct atm_dev_addr), GFP_ATOMIC);
+	this = kmalloc(sizeof(*this), GFP_ATOMIC);
 	if (!this) {
 		spin_unlock_irqrestore(&dev->lock, flags);
 		return -ENOMEM;
diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 4e111196f902..f5b601c01f38 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -538,7 +538,7 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 
 	if (copy_from_user(&be, arg, sizeof be))
 		return -EFAULT;
-	brvcc = kzalloc(sizeof(struct br2684_vcc), GFP_KERNEL);
+	brvcc = kzalloc(sizeof(*brvcc), GFP_KERNEL);
 	if (!brvcc)
 		return -ENOMEM;
 	/*
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 65f706e4344c..041d519b8771 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -60,7 +60,7 @@ static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
 	skb = alloc_skb(sizeof(struct atmarp_ctrl), GFP_ATOMIC);
 	if (!skb)
 		return -ENOMEM;
-	ctrl = skb_put(skb, sizeof(struct atmarp_ctrl));
+	ctrl = skb_put(skb, sizeof(*ctrl));
 	ctrl->type = type;
 	ctrl->itf_num = itf;
 	ctrl->ip = ip;
@@ -418,7 +418,7 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
 
 	if (!vcc->push)
 		return -EBADFD;
-	clip_vcc = kmalloc(sizeof(struct clip_vcc), GFP_KERNEL);
+	clip_vcc = kmalloc(sizeof(*clip_vcc), GFP_KERNEL);
 	if (!clip_vcc)
 		return -ENOMEM;
 	pr_debug("%p vcc %p\n", clip_vcc, vcc);
diff --git a/net/atm/lec.c b/net/atm/lec.c
index f5be0b931978..74a794602412 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -690,7 +690,7 @@ static int lec_vcc_attach(struct atm_vcc *vcc, void __user *arg)
 	if (ioc_data.dev_num < 0 || ioc_data.dev_num >= MAX_LEC_ITF ||
 	    !dev_lec[ioc_data.dev_num])
 		return -EINVAL;
-	vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+	vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
 	if (!vpriv)
 		return -ENOMEM;
 	vpriv->xoff = 0;
@@ -1552,7 +1552,7 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
 {
 	struct lec_arp_table *to_return;
 
-	to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
+	to_return = kzalloc(sizeof(*to_return), GFP_ATOMIC);
 	if (!to_return)
 		return NULL;
 
@@ -2155,7 +2155,7 @@ static int lec_mcast_make(struct lec_priv *priv, struct atm_vcc *vcc)
 	struct lec_vcc_priv *vpriv;
 	int err = 0;
 
-	vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+	vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
 	if (!vpriv)
 		return -ENOMEM;
 	vpriv->xoff = 0;
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index dd57d05b5dcc..d6729d797107 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -183,7 +183,7 @@ struct atm_mpoa_qos *atm_mpoa_add_qos(__be32 dst_ip, struct atm_qos *qos)
 		return entry;
 	}
 
-	entry = kmalloc(sizeof(struct atm_mpoa_qos), GFP_KERNEL);
+	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return entry;
 
@@ -279,7 +279,7 @@ static struct mpoa_client *alloc_mpc(void)
 {
 	struct mpoa_client *mpc;
 
-	mpc = kzalloc(sizeof(struct mpoa_client), GFP_KERNEL);
+	mpc = kzalloc(sizeof(*mpc), GFP_KERNEL);
 	if (mpc == NULL)
 		return NULL;
 	rwlock_init(&mpc->ingress_lock);
diff --git a/net/atm/mpoa_caches.c b/net/atm/mpoa_caches.c
index 7495b42d59eb..23f36e5a20ee 100644
--- a/net/atm/mpoa_caches.c
+++ b/net/atm/mpoa_caches.c
@@ -96,7 +96,7 @@ static in_cache_entry *in_cache_get_by_vcc(struct atm_vcc *vcc,
 static in_cache_entry *in_cache_add_entry(__be32 dst_ip,
 					  struct mpoa_client *client)
 {
-	in_cache_entry *entry = kzalloc(sizeof(in_cache_entry), GFP_KERNEL);
+	in_cache_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 
 	if (!entry)
 		return NULL;
@@ -456,7 +456,7 @@ static void eg_cache_remove_entry(eg_cache_entry *entry,
 static eg_cache_entry *eg_cache_add_entry(struct k_message *msg,
 					  struct mpoa_client *client)
 {
-	eg_cache_entry *entry = kzalloc(sizeof(eg_cache_entry), GFP_KERNEL);
+	eg_cache_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 
 	if (!entry)
 		return NULL;
diff --git a/net/atm/signaling.c b/net/atm/signaling.c
index 0a20f6e953ac..225234f5cb5f 100644
--- a/net/atm/signaling.c
+++ b/net/atm/signaling.c
@@ -150,7 +150,7 @@ void sigd_enq2(struct atm_vcc *vcc, enum atmsvc_msg_type type,
 	pr_debug("%d (0x%p)\n", (int)type, vcc);
 	while (!(skb = alloc_skb(sizeof(struct atmsvc_msg), GFP_KERNEL)))
 		schedule();
-	msg = skb_put_zero(skb, sizeof(struct atmsvc_msg));
+	msg = skb_put_zero(skb, sizeof(*msg));
 	msg->type = type;
 	*(struct atm_vcc **) &msg->vcc = vcc;
 	*(struct atm_vcc **) &msg->listen_vcc = listen_vcc;
-- 
2.14.2

^ permalink raw reply related

* [PATCH 1/3] net/atm: Delete an error message for a failed memory allocation in five functions
From: SF Markus Elfring @ 2017-10-09 20:49 UTC (permalink / raw)
  To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
	Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
	Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
	Mitchell Blank Jr, Roopa Prabhu
  Cc: LKML, kernel-janitors
In-Reply-To: <10a92558-1105-b947-86a2-6ac763cca36d@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 21:34:35 +0200

Omit extra messages for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c         | 5 ++---
 net/atm/mpc.c         | 8 ++------
 net/atm/mpoa_caches.c | 8 ++------
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index a3d93a1bb133..f5be0b931978 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1553,10 +1553,9 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
 	struct lec_arp_table *to_return;
 
 	to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
-	if (!to_return) {
-		pr_info("LEC: Arp entry kmalloc failed\n");
+	if (!to_return)
 		return NULL;
-	}
+
 	ether_addr_copy(to_return->mac_addr, mac_addr);
 	INIT_HLIST_NODE(&to_return->next);
 	setup_timer(&to_return->timer, lec_arp_expire_arp,
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index 5677147209e8..dd57d05b5dcc 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -184,10 +184,8 @@ struct atm_mpoa_qos *atm_mpoa_add_qos(__be32 dst_ip, struct atm_qos *qos)
 	}
 
 	entry = kmalloc(sizeof(struct atm_mpoa_qos), GFP_KERNEL);
-	if (entry == NULL) {
-		pr_info("mpoa: out of memory\n");
+	if (!entry)
 		return entry;
-	}
 
 	entry->ipaddr = dst_ip;
 	entry->qos = *qos;
@@ -473,10 +471,8 @@ static const uint8_t *copy_macs(struct mpoa_client *mpc,
 			kfree(mpc->mps_macs);
 		mpc->number_of_mps_macs = 0;
 		mpc->mps_macs = kmalloc(num_macs * ETH_ALEN, GFP_KERNEL);
-		if (mpc->mps_macs == NULL) {
-			pr_info("(%s) out of mem\n", mpc->dev->name);
+		if (!mpc->mps_macs)
 			return NULL;
-		}
 	}
 	ether_addr_copy(mpc->mps_macs, router_mac);
 	tlvs += 20; if (device_type == MPS_AND_MPC) tlvs += 20;
diff --git a/net/atm/mpoa_caches.c b/net/atm/mpoa_caches.c
index 4ccaa16b1eb1..7495b42d59eb 100644
--- a/net/atm/mpoa_caches.c
+++ b/net/atm/mpoa_caches.c
@@ -98,10 +98,8 @@ static in_cache_entry *in_cache_add_entry(__be32 dst_ip,
 {
 	in_cache_entry *entry = kzalloc(sizeof(in_cache_entry), GFP_KERNEL);
 
-	if (entry == NULL) {
-		pr_info("mpoa: mpoa_caches.c: new_in_cache_entry: out of memory\n");
+	if (!entry)
 		return NULL;
-	}
 
 	dprintk("adding an ingress entry, ip = %pI4\n", &dst_ip);
 
@@ -460,10 +458,8 @@ static eg_cache_entry *eg_cache_add_entry(struct k_message *msg,
 {
 	eg_cache_entry *entry = kzalloc(sizeof(eg_cache_entry), GFP_KERNEL);
 
-	if (entry == NULL) {
-		pr_info("out of memory\n");
+	if (!entry)
 		return NULL;
-	}
 
 	dprintk("adding an egress entry, ip = %pI4, this should be our IP\n",
 		&msg->content.eg_info.eg_dst_ip);
-- 
2.14.2

^ permalink raw reply related

* [PATCH 0/3] net-ATM: Adjustments for several function implementations
From: SF Markus Elfring @ 2017-10-09 20:48 UTC (permalink / raw)
  To: netdev, Alexey Dobriyan, Andrew Morton, Augusto Mecking Caringi,
	Bhumika Goyal, David S. Miller, David Windsor, Elena Reshetova,
	Hans Liljestrand, Jarod Wilson, Johannes Berg, Kees Cook,
	Mitchell Blank Jr, Roopa Prabhu
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 9 Oct 2017 22:30:33 +0200

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in five functions
  Improve a size determination in 12 functions
  Adjust 121 checks for null pointers

 net/atm/addr.c        |   2 +-
 net/atm/br2684.c      |  19 ++++----
 net/atm/clip.c        |   6 +--
 net/atm/lec.c         |  49 ++++++++++----------
 net/atm/mpc.c         | 126 ++++++++++++++++++++++++--------------------------
 net/atm/mpoa_caches.c |  75 +++++++++++++++---------------
 net/atm/pppoatm.c     |  11 ++---
 net/atm/proc.c        |   2 +-
 net/atm/signaling.c   |   2 +-
 9 files changed, 141 insertions(+), 151 deletions(-)

-- 
2.14.2

^ permalink raw reply

* [net-next 3/3] ip_gre: cache the device mtu hard_header_len calc
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
  To: netdev; +Cc: David Laight
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>

The patch introduces ip_tunnel->ether_mtu fields to cache the value of
dev->mtu + dev->hard_header_len.  This avoids the arithmetic operation
on every packet.

Signed-off-by: William Tu <u9012063@gmail.com>
Cc: David Laight <David.Laight@aculab.com>
---
 include/net/ip_tunnels.h | 1 +
 net/ipv4/ip_gre.c        | 8 ++++----
 net/ipv4/ip_tunnel.c     | 3 +++
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index b41a1e057fce..19565be26e13 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -117,6 +117,7 @@ struct ip_tunnel {
 
 	/* This field used only by ERSPAN */
 	u32		index;		/* ERSPAN type II index */
+	unsigned int	ether_mtu;	/* The mtu including the ether hdr */
 
 	struct dst_cache dst_cache;
 
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 6e6e4c4811cc..994b8ddea0b1 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -578,8 +578,8 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 	if (gre_handle_offloads(skb, false))
 		goto err_free_rt;
 
-	if (skb->len > dev->mtu + dev->hard_header_len) {
-		pskb_trim(skb, dev->mtu + dev->hard_header_len);
+	if (skb->len > tunnel->ether_mtu) {
+		pskb_trim(skb, tunnel->ether_mtu);
 		truncate = true;
 	}
 
@@ -730,8 +730,8 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
 	if (skb_cow_head(skb, dev->needed_headroom))
 		goto free_skb;
 
-	if (skb->len > dev->mtu + dev->hard_header_len) {
-		pskb_trim(skb, dev->mtu + dev->hard_header_len);
+	if (skb->len > tunnel->ether_mtu) {
+		pskb_trim(skb, tunnel->ether_mtu);
 		truncate = true;
 	}
 
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index fe6fee728ce4..859af5b86802 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -348,6 +348,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
 
 	dev->needed_headroom = t_hlen + hlen;
 	mtu -= (dev->hard_header_len + t_hlen);
+	tunnel->ether_mtu = mtu + dev->hard_header_len;
 
 	if (mtu < 68)
 		mtu = 68;
@@ -952,6 +953,7 @@ int __ip_tunnel_change_mtu(struct net_device *dev, int new_mtu, bool strict)
 	}
 
 	dev->mtu = new_mtu;
+	tunnel->ether_mtu = new_mtu + dev->hard_header_len;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__ip_tunnel_change_mtu);
@@ -1183,6 +1185,7 @@ int ip_tunnel_init(struct net_device *dev)
 
 	tunnel->dev = dev;
 	tunnel->net = dev_net(dev);
+	tunnel->ether_mtu = dev->mtu + dev->hard_header_len;
 	strcpy(tunnel->parms.name, dev->name);
 	iph->version		= 4;
 	iph->ihl		= 5;
-- 
2.7.4

^ permalink raw reply related

* [net-next 2/3] ip_gre: fix erspan tunnel mtu calculation
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
  To: netdev; +Cc: Xin Long
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>

Remove the unnecessary -4 and +4 bytes at mtu and headroom calculation.
In addition, erspan uses fixed 8-byte gre header, so add ERSPAN_GREHDR_LEN
macro for better readability.

Now tunnel->hlen = grehdr(8) + erspanhdr(8) = 16 byte.
The mtu should be ETH_DATA_LEN - 16 - iph(20) = 1464.
After the ip_tunnel_bind_dev(), the mtu is adjusted to
1464 - 14 (dev->hard_header_len) = 1450.
The maximum skb->len the erspan tunnel can carry without
being truncated is 1450 + 14 = 1464 byte.

Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Xin Long <lucien.xin@gmail.com>
---
 include/net/erspan.h |  1 +
 net/ipv4/ip_gre.c    | 11 +++++------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/erspan.h b/include/net/erspan.h
index ca94fc86865e..e28294e248d0 100644
--- a/include/net/erspan.h
+++ b/include/net/erspan.h
@@ -28,6 +28,7 @@
  */
 
 #define ERSPAN_VERSION	0x1
+#define ERSPAN_GREHDR_LEN 8	/* ERSPAN has fixed 8-byte GRE header */
 
 #define VER_MASK	0xf000
 #define VLAN_MASK	0x0fff
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 286065c35959..6e6e4c4811cc 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -569,8 +569,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	key = &tun_info->key;
 
-	/* ERSPAN has fixed 8 byte GRE header */
-	tunnel_hlen = 8 + sizeof(struct erspanhdr);
+	tunnel_hlen = ERSPAN_GREHDR_LEN + sizeof(struct erspanhdr);
 
 	rt = prepare_fb_xmit(skb, dev, &fl, tunnel_hlen);
 	if (!rt)
@@ -591,7 +590,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 	erspan_build_header(skb, tunnel_id_to_key32(key->tun_id),
 			    ntohl(md->index), truncate);
 
-	gre_build_header(skb, 8, TUNNEL_SEQ,
+	gre_build_header(skb, ERSPAN_GREHDR_LEN, TUNNEL_SEQ,
 			 htons(ETH_P_ERSPAN), 0, htonl(tunnel->o_seqno++));
 
 	df = key->tun_flags & TUNNEL_DONT_FRAGMENT ?  htons(IP_DF) : 0;
@@ -1242,14 +1241,14 @@ static int erspan_tunnel_init(struct net_device *dev)
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	int t_hlen;
 
-	tunnel->tun_hlen = 8;
+	tunnel->tun_hlen = ERSPAN_GREHDR_LEN;
 	tunnel->parms.iph.protocol = IPPROTO_GRE;
 	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen +
 		       sizeof(struct erspanhdr);
 	t_hlen = tunnel->hlen + sizeof(struct iphdr);
 
-	dev->needed_headroom = LL_MAX_HEADER + t_hlen + 4;
-	dev->mtu = ETH_DATA_LEN - t_hlen - 4;
+	dev->needed_headroom = LL_MAX_HEADER + t_hlen;
+	dev->mtu = ETH_DATA_LEN - t_hlen;
 	dev->features		|= GRE_FEATURES;
 	dev->hw_features	|= GRE_FEATURES;
 	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE;
-- 
2.7.4

^ permalink raw reply related

* [net-next 1/3] ip_gre: fix mtu and headroom size
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
  To: netdev; +Cc: Tom Herbert
In-Reply-To: <1507582067-36718-1-git-send-email-u9012063@gmail.com>

The ip_gre_calc_hlen() already counts gre's base header len (4-byte)
plus optional header's len, so tunnel->hlen has the entire gre
headeri + options len. Thus, remove the -4 and +4 when calculating
the needed_headroom and mtu.

Fixes: 4565e9919cda ("gre: Setup and TX path for gre/UDP foo-over-udp encapsulation")
Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Tom Herbert <tom@quantonium.net>
---
 net/ipv4/ip_gre.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c105a315b1a3..286065c35959 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -947,8 +947,8 @@ static void __gre_tunnel_init(struct net_device *dev)
 
 	t_hlen = tunnel->hlen + sizeof(struct iphdr);
 
-	dev->needed_headroom	= LL_MAX_HEADER + t_hlen + 4;
-	dev->mtu		= ETH_DATA_LEN - t_hlen - 4;
+	dev->needed_headroom	= LL_MAX_HEADER + t_hlen;
+	dev->mtu		= ETH_DATA_LEN - t_hlen;
 
 	dev->features		|= GRE_FEATURES;
 	dev->hw_features	|= GRE_FEATURES;
-- 
2.7.4

^ permalink raw reply related

* [net-next 0/3] ip_gre: a bunch of fixes for mtu
From: William Tu @ 2017-10-09 20:47 UTC (permalink / raw)
  To: netdev

The first two patches are to fix some issues for mtu and needed_headroom length
calculation from the gre and erspan tunnel header.  The last path tries to avoid
arithmetic operation for every packet when checking for erspan truncate.

William Tu (3):
  ip_gre: fix mtu and headroom size
  ip_gre: fix erspan tunnel mtu calculation
  ip_gre: cache the device mtu hard_header_len calc

 include/net/erspan.h     |  1 +
 include/net/ip_tunnels.h |  1 +
 net/ipv4/ip_gre.c        | 23 +++++++++++------------
 net/ipv4/ip_tunnel.c     |  3 +++
 4 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH] once: switch to new jump label API
From: Eric Biggers @ 2017-10-09 20:27 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S. Miller, netdev, linux-kernel, Jason Baron,
	Peter Zijlstra, Eric Biggers
In-Reply-To: <20170916040751.GA14238@zzz.localdomain>

On Fri, Sep 15, 2017 at 09:07:51PM -0700, Eric Biggers wrote:
> On Tue, Aug 22, 2017 at 02:44:41PM -0400, Hannes Frederic Sowa wrote:
> > Eric Biggers <ebiggers3@gmail.com> writes:
> > 
> > > From: Eric Biggers <ebiggers@google.com>
> > >
> > > Switch the DO_ONCE() macro from the deprecated jump label API to the new
> > > one.  The new one is more readable, and for DO_ONCE() it also makes the
> > > generated code more icache-friendly: now the one-time initialization
> > > code is placed out-of-line at the jump target, rather than at the inline
> > > fallthrough case.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > 
> > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org.
> > 
> > Thanks!
> 
> Great!  Who though is the maintainer for this code?  It seems it was originally
> taken by David Miller through the networking tree.  David, are you taking
> further patches to the "once" functions, or should I be trying to get this into
> -mm, or somewhere else?
> 
> Eric

Ping.

^ permalink raw reply

* Re: [PATCH v2] net/core: Fix BUG to BUG_ON conditionals.
From: Levin, Alexander (Sasha Levin) @ 2017-10-09 20:26 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Tim Hansen, davem@davemloft.net, willemb@google.com,
	edumazet@google.com, soheil@google.com, pabeni@redhat.com,
	elena.reshetova@intel.com, tom@quantonium.net, Jason@zx2c4.com,
	fw@strlen.de, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20171009171540.lgmk2slq3bdrptxk@ast-mbp.dhcp.thefacebook.com>

On Mon, Oct 09, 2017 at 10:15:42AM -0700, Alexei Starovoitov wrote:
>On Mon, Oct 09, 2017 at 11:37:59AM -0400, Tim Hansen wrote:
>> Fix BUG() calls to use BUG_ON(conditional) macros.
>>
>> This was found using make coccicheck M=net/core on linux next
>> tag next-2017092
>>
>> Signed-off-by: Tim Hansen <devtimhansen@gmail.com>
>> ---
>>  net/core/skbuff.c | 15 ++++++---------
>>  1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index d98c2e3ce2bf..34ce4c1a0f3c 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -1350,8 +1350,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
>>  	/* Set the tail pointer and length */
>>  	skb_put(n, skb->len);
>>
>> -	if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
>> -		BUG();
>> +	BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
>
>I'm concerned with this change.
>1. Calling non-trivial bit of code inside the macro is a poor coding style (imo)
>2. BUG_ON != BUG. Some archs like mips and ppc have HAVE_ARCH_BUG_ON and implementation
>of BUG and BUG_ON look quite different.

For these archs, wouldn't it then be more efficient to use BUG_ON rather than BUG()?

-- 

Thanks,
Sasha

^ permalink raw reply

* Re: [PATCHv4 iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Phil Sutter @ 2017-10-09 20:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Hangbin Liu, netdev, Michal Kubecek, Hangbin Liu
In-Reply-To: <20171002103708.0572704b@xeon-e3>

Hi Stephen,

On Mon, Oct 02, 2017 at 10:37:08AM -0700, Stephen Hemminger wrote:
> On Thu, 28 Sep 2017 21:33:46 +0800
> Hangbin Liu <haliu@redhat.com> wrote:
> 
> > From: Hangbin Liu <liuhangbin@gmail.com>
> > 
> > This is an update for 460c03f3f3cc ("iplink: double the buffer size also in
> > iplink_get()"). After update, we will not need to double the buffer size
> > every time when VFs number increased.
> > 
> > With call like rtnl_talk(&rth, &req.n, NULL, 0), we can simply remove the
> > length parameter.
> > 
> > With call like rtnl_talk(&rth, nlh, nlh, sizeof(req), I add a new variable
> > answer to avoid overwrite data in nlh, because it may has more info after
> > nlh. also this will avoid nlh buffer not enough issue.
> > 
> > We need to free answer after using.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> 
> Most of the uses of rtnl_talk() don't need to this peek and dynamic sizing.
> Can only those places that need that be targeted?

We could probably do that, by having a buffer on stack in __rtnl_talk()
which will be used instead of the allocated one if 'answer' is NULL. Or
maybe even introduce a dedicated API call for the dynamically allocated
receive buffer. But I really doubt that's feasible: AFAICT, that stack
buffer still needs to be reasonably sized since the reply might be
larger than the request (reusing the request buffer would be the most
simple way to tackle this), also there is support for extack which may
bloat the response to arbitrary size. Hangbin has shown in his benchmark
that the overhead of the second syscall is negligible, so why care about
that and increase code complexity even further?

Not saying it's not possible, but I just doubt it's worth the effort.

Cheers, Phil

^ permalink raw reply

* RE: [net-next 14/15] i40e: ignore skb->xmit_more when deciding to set RS bit
From: Keller, Jacob E @ 2017-10-09 20:10 UTC (permalink / raw)
  To: Alexander Duyck, David Laight
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <CAKgT0UfZEU++hZkpp1NixYxj=2STfXX-5opz2BoEhG1SfUA5pQ@mail.gmail.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Alexander Duyck
> Sent: Monday, October 09, 2017 9:28 AM
> To: David Laight <David.Laight@aculab.com>
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net;
> Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org;
> nhorman@redhat.com; sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 14/15] i40e: ignore skb->xmit_more when deciding to set
> RS bit
> 
> On Mon, Oct 9, 2017 at 2:07 AM, David Laight <David.Laight@aculab.com> wrote:
> > From: Jeff Kirsher
> >> Sent: 06 October 2017 18:57
> >> From: Jacob Keller <jacob.e.keller@intel.com>
> >>
> >> Since commit 6a7fded776a7 ("i40e: Fix RS bit update in Tx path and
> >> disable force WB workaround") we've tried to "optimize" setting the
> >> RS bit based around skb->xmit_more. This same logic was refactored
> >> in commit 1dc8b538795f ("i40e: Reorder logic for coalescing RS bits"),
> >> but ultimately was not functionally changed.
> >
> > I've tried to understand the above, but without definitions of WB
> > and RS and the '4-ness' of something it is quite difficult.
> >
> > I THINK this is all about telling the hardware there is a packet
> > in the ring to transmit?
> >
> > I don't understand the 4-ness. Linux requires that the hardware
> > be notified of a single packet transmit, and that the 'transmit
> > complete' also be notified in 'a timely manner' for a single packet.
> 
> So to clarify some of this. The RS is short for Report Status. It
> tells the hardware that when it has completed the descriptor it should
> trigger a write back, aka WB.
> 
> The 4-ness is because each descriptor is 16 bytes, so if we write back
> once every 4 descriptors that is one 64B cache line which is much
> better for most platforms performance wise.
> 
> > skb->xmit_more ought to be usable to optimise both of these
> > (assuming it isn't a lie).
> 
> Actually it leads to issues if we hold off for too long. If we are
> busy dequeueing a long list, and the Tx queue has gone empty then we
> are stalling Tx without need to do so. We need to be regularly
> notifying the device that it has buffers available to transmit which
> is what xmit_more is good for. However in addition the hardware needs
> to notify us regularly that there are buffers ready to clean which it
> isn't necessarily so useful for, especially if  are filling the entire
> ring and only providing one notification to the driver that there are
> buffers ready since we can defer the Tx interrupt for too long.
> 
> What Jake is trying to fix here is to prevent us from generating what
> looks like a square wave in terms of throughput. Essentially the
> current behavior that is being seen is that all the buffers in the
> ring either belong to software, or belong to hardware. It is best for
> us to have this split half and half so that the hardware has buffers
> to transmit and software has buffers to clean and enqueue new buffers
> onto.
> 

Yes this sums it up pretty well. The test case which found this bug is outlined in the description. Essentially we could see up to dozens or even almost a hundred packets in a row bunched up if we had a bunch of virtual devices layered on top of the network driver. This led to us not indicating to get status from the hardware about finished packets, which results in an increased delay to finish Tx.

> > The driver would need to ensure a ring full of messages isn't
> > generated without either wakeup - but that might be 128 frames.
> 
> That is what is happening here. Basically we are guaranteeing
> writebacks are occurring on a regular basis instead of in large
> bursts.
> 
> > FWIW on the systems I have PCIe writes are relatively cheap
> > (reads are slow). So different counts would be appropriate
> > for delaying doorbell writes and requesting completion interrupts.
> >
> >         David
> 
> The doorbell writes are still being delayed the same amount with this
> patch. The only thing that was split out was the device write backs
> are now occurring on a more regular basis instead of being held off
> until a much larger set is handled.
> 

Exactly. The tail bumps are still functioning the same, but it's the hardware report status requests which are not delayed. Note that there's some further quirks in how our hardware performs write backs which exacerbate the problem because of how the cachelines are setup so that the delay is even worse than we would expect.

Thanks,
Jake

> - Alex

^ permalink raw reply

* Re: [PATCH v1 RFC 6/7] Add MIB counter reading support
From: Florian Fainelli @ 2017-10-09 20:07 UTC (permalink / raw)
  To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-7-git-send-email-Tristram.Ha@microchip.com>

On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Add MIB counter reading support.
> Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product
> name is always KSZ####.
> Header file ksz_priv.h no longer contains any chip specific data.

You should probably explain in the commit message that you are adding a
timer that reads counters every 30 seconds to avoid overflows, as this
is a pretty important piece of information.


> +static void ksz9477_phy_setup(struct ksz_device *dev, int port,
> +			      struct phy_device *phy)
> +{
> +	if (port < dev->phy_port_cnt) {
> +		/* SUPPORTED_Asym_Pause and SUPPORTED_Pause can be removed to
> +		 * disable flow control when rate limiting is used.
> +		 */
> +		phy->advertising = phy->supported;
> +	}
> +}

This appears to be an unrelated change here that you would want to put
in a separate patch.

> +
>  static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
>  {
>  	u8 data8;
> @@ -1159,6 +1198,8 @@ static int ksz9477_setup(struct dsa_switch *ds)
>  	/* start switch */
>  	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
>  
> +	ksz_init_mib_timer(dev);
> +
>  	return 0;
>  }
>  
> @@ -1168,6 +1209,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
>  	.set_addr		= ksz9477_set_addr,
>  	.phy_read		= ksz9477_phy_read16,
>  	.phy_write		= ksz9477_phy_write16,
> +	.adjust_link		= ksz_adjust_link,
>  	.port_enable		= ksz_enable_port,
>  	.port_disable		= ksz_disable_port,
>  	.get_strings		= ksz9477_get_strings,
> @@ -1289,6 +1331,7 @@ static int ksz9477_switch_init(struct ksz_device *dev)
>  	if (!dev->ports)
>  		return -ENOMEM;
>  	for (i = 0; i < dev->mib_port_cnt; i++) {
> +		mutex_init(&dev->ports[i].mib.cnt_mutex);
>  		dev->ports[i].mib.counters =
>  			devm_kzalloc(dev->dev,
>  				     sizeof(u64) *
> @@ -1310,7 +1353,12 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
>  	.get_port_addr = ksz9477_get_port_addr,
>  	.cfg_port_member = ksz9477_cfg_port_member,
>  	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
> +	.phy_setup = ksz9477_phy_setup,
>  	.port_setup = ksz9477_port_setup,
> +	.r_mib_cnt = ksz9477_r_mib_cnt,
> +	.r_mib_pkt = ksz9477_r_mib_pkt,
> +	.freeze_mib = ksz9477_freeze_mib,
> +	.port_init_cnt = ksz9477_port_init_cnt,
>  	.shutdown = ksz9477_reset_switch,
>  	.detect = ksz9477_switch_detect,
>  	.init = ksz9477_switch_init,
> diff --git a/drivers/net/dsa/microchip/ksz_9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
> similarity index 100%
> rename from drivers/net/dsa/microchip/ksz_9477_reg.h
> rename to drivers/net/dsa/microchip/ksz9477_reg.h
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 1c9c4c5..885b3e3 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -50,6 +50,81 @@ void ksz_update_port_member(struct ksz_device *dev, int port)
>  	}
>  }
>  
> +static void port_r_cnt(struct ksz_device *dev, int port)
> +{
> +	struct ksz_port_mib *mib = &dev->ports[port].mib;
> +	u64 *dropped;
> +
> +	/* Some ports may not have MIB counters before SWITCH_COUNTER_NUM. */
> +	while (mib->cnt_ptr < dev->reg_mib_cnt) {
> +		dev->dev_ops->r_mib_cnt(dev, port, mib->cnt_ptr,
> +					&mib->counters[mib->cnt_ptr]);
> +		++mib->cnt_ptr;
> +	}
> +
> +	/* last one in storage */
> +	dropped = &mib->counters[dev->mib_cnt];
> +
> +	/* Some ports may not have MIB counters after SWITCH_COUNTER_NUM. */
> +	while (mib->cnt_ptr < dev->mib_cnt) {
> +		dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
> +					dropped, &mib->counters[mib->cnt_ptr]);
> +		++mib->cnt_ptr;
> +	}
> +	mib->cnt_ptr = 0;
> +}
> +
> +static void ksz_mib_read_work(struct work_struct *work)
> +{
> +	struct ksz_device *dev =
> +		container_of(work, struct ksz_device, mib_read);
> +	struct ksz_port *p;
> +	struct ksz_port_mib *mib;
> +	int i;
> +
> +	for (i = 0; i < dev->mib_port_cnt; i++) {
> +		p = &dev->ports[i];
> +		if (!p->on)
> +			continue;
> +		mib = &p->mib;
> +		mutex_lock(&mib->cnt_mutex);
> +
> +		/* read only dropped counters when link is not up */
> +		if (p->link_down)
> +			p->link_down = 0;
> +		else if (!p->link_up)
> +			mib->cnt_ptr = dev->reg_mib_cnt;

Can't you just check the ports' PHY device here instead of caching that
(which can get out of sync)?


> +void ksz_adjust_link(struct dsa_switch *ds, int port,
> +		     struct phy_device *phydev)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct ksz_port *p = &dev->ports[port];
> +
> +	if (phydev->link) {
> +		p->speed = phydev->speed;
> +		p->duplex = phydev->duplex;
> +		p->flow_ctrl = phydev->pause;
> +		p->link_up = 1;
> +		dev->live_ports |= (1 << port) & dev->on_ports;
> +	} else if (p->link_up) {
> +		p->link_up = 0;
> +		p->link_down = 1;
> +		dev->live_ports &= ~(1 << port);
> +	}
> +}

It would have been nice to explain why this is needed in the commit
message. dev->live_ports is read-only it seems?
-- 
Florian

^ permalink raw reply

* Re: [PATCH v1 RFC 5/7] Break KSZ9477 DSA driver into two files
From: Florian Fainelli @ 2017-10-09 20:01 UTC (permalink / raw)
  To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-6-git-send-email-Tristram.Ha@microchip.com>

On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Break KSZ9477 DSA driver into two files in preparation to add more KSZ
> switch drivers.
> Add common functions in ksz_common.h so that other KSZ switch drivers
> can access code in ksz_common.c.
> Add ksz_spi.h for common functions used by KSZ switch SPI drivers.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* RE: [PATCH v1 RFC 1/7] Replace license with GPL
From: Woojung.Huh @ 2017-10-09 19:58 UTC (permalink / raw)
  To: Tristram.Ha, andrew, f.fainelli, pavel, ruediger.schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-2-git-send-email-Tristram.Ha@microchip.com>

> Subject: [PATCH v1 RFC 1/7] Replace license with GPL
> 
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Replace license with GPL.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>

Reviewed-by: Woojung Huh <Woojung.Huh@microchip.com>

^ permalink raw reply

* Re: [PATCH v1 RFC 0/7] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers
From: Florian Fainelli @ 2017-10-09 19:58 UTC (permalink / raw)
  To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-1-git-send-email-Tristram.Ha@microchip.com>

On 10/06/2017 01:32 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> This series of patches is to modify the original KSZ9477 DSA driver so
> that other KSZ switch drivers can be added and use the common code.
> 
> There are several steps to accomplish this achievement.  First is to
> rename some function names with a prefix to indicate chip specific
> function.  Second is to move common code into header that can be shared.
> Last is to modify tag_ksz.c so that it can handle many tail tag formats
> used by different KSZ switch drivers.
> 
> ksz_common.c will contain the common code used by all KSZ switch drivers.
> ksz9477.c will contain KSZ9477 code from the original ksz_common.c.
> ksz9477_spi.c is renamed from ksz_spi.c.
> ksz9477_reg.h is renamed from ksz_9477_reg.h.
> ksz_common.h is added to provide common code access to KSZ switch
> drivers.
> ksz_spi.h is added to provide common SPI access functions to KSZ SPI
> drivers.

Most of this looks fine, and it is probably time to move away from the
RFC state and do a formal patch submission with the Reviewed-by and
Acked-by tags you just collected.

Can you also make sure you prefix your patches with: net: dsa: to be
consistent with other submissions done to that subsystem, e.g:

net: dsa: microchip: Replace license with GPL

Thank you!

> 
> v1
> - Each patch in the set is self-contained
> - Use ksz9477 prefix to indicate KSZ9477 specific code
> - Simplify MIB counter reading code
> - Switch driver code is not accessed from tag_ksz.c
> 
> Tristram Ha (7):
>   Replace license with GPL.
>   Clean up code according to patch check suggestions.
>   Rename some functions with ksz9477 prefix to separate chip specific
>     code from common code.
>   Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to
>     add more KSZ switch drivers.
>   Break KSZ9477 DSA driver into two files in preparation to add more KSZ
>     switch drivers.  Add common functions in ksz_common.h so that other
>     KSZ switch drivers can access code in ksz_common.c.  Add ksz_spi.h
>     for common functions used by KSZ switch SPI drivers.
>   Add MIB counter reading support.  Rename ksz_9477_reg.h to
>     ksz9477_reg.h for consistency as the product name is always KSZ####.
>     Header file ksz_priv.h no longer contains any chip specific data.
>   Modify tag_ksz.c so that tail tag code can be used by other KSZ switch
>     drivers.
> 
>  drivers/net/dsa/microchip/Kconfig                  |   14 +-
>  drivers/net/dsa/microchip/Makefile                 |    4 +-
>  drivers/net/dsa/microchip/ksz9477.c                | 1376 ++++++++++++++++++++
>  .../microchip/{ksz_9477_reg.h => ksz9477_reg.h}    |   23 +-
>  drivers/net/dsa/microchip/ksz9477_spi.c            |  188 +++
>  drivers/net/dsa/microchip/ksz_common.c             | 1210 ++++-------------
>  drivers/net/dsa/microchip/ksz_common.h             |  234 ++++
>  drivers/net/dsa/microchip/ksz_priv.h               |  258 ++--
>  drivers/net/dsa/microchip/ksz_spi.c                |  216 ---
>  drivers/net/dsa/microchip/ksz_spi.h                |   82 ++
>  include/net/dsa.h                                  |    2 +-
>  net/dsa/Kconfig                                    |    4 +
>  net/dsa/dsa.c                                      |    4 +-
>  net/dsa/dsa_priv.h                                 |    2 +-
>  net/dsa/tag_ksz.c                                  |  107 +-
>  15 files changed, 2331 insertions(+), 1393 deletions(-)
>  create mode 100644 drivers/net/dsa/microchip/ksz9477.c
>  rename drivers/net/dsa/microchip/{ksz_9477_reg.h => ksz9477_reg.h} (98%)
>  create mode 100644 drivers/net/dsa/microchip/ksz9477_spi.c
>  create mode 100644 drivers/net/dsa/microchip/ksz_common.h
>  delete mode 100644 drivers/net/dsa/microchip/ksz_spi.c
>  create mode 100644 drivers/net/dsa/microchip/ksz_spi.h
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH v1 RFC 4/7] Rename ksz_spi.c to ksz9477_spi.c
From: Florian Fainelli @ 2017-10-09 19:56 UTC (permalink / raw)
  To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-5-git-send-email-Tristram.Ha@microchip.com>

On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to add
> more KSZ switch drivers.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: linux-next: manual merge of the drivers-x86 tree with the net-next tree
From: Mark Brown @ 2017-10-09 19:56 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Darren Hart, Mario Limonciello, Yehezkel Bernat, Andy Shevchenko,
	Amir Levy, Michael Jamet, David S. Miller, netdev,
	Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20171009194301.GP2761@lahna.fi.intel.com>

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

On Mon, Oct 09, 2017 at 10:43:01PM +0300, Mika Westerberg wrote:

> If possible, I would rather move this chapter to be before "Networking
> over Thunderbolt cable". Reason is that it then follows NVM flashing
> chapter which is typically where you need to force power in the first
> place.

I guess that's something best sorted out either in the relevant trees or
during the merge window?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v1 RFC 3/7] Rename some functions with ksz9477 prefix
From: Florian Fainelli @ 2017-10-09 19:56 UTC (permalink / raw)
  To: Tristram.Ha, Andrew Lunn, Pavel Machek, Ruediger Schmitt
  Cc: muvarov, nathan.leigh.conrad, vivien.didelot, UNGLinuxDriver,
	netdev, linux-kernel
In-Reply-To: <1507321985-15097-4-git-send-email-Tristram.Ha@microchip.com>

On 10/06/2017 01:33 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Rename some functions with ksz9477 prefix to separate chip specific code
> from common code.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply


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