lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 08 Apr 2014 10:14:12 -0700 From: Joe Perches <joe@...ches.com> To: Stephen Hemminger <stephen@...workplumber.org> Cc: Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org Subject: Re: [PATCH 2/2] macvlan: Move broadcasts into a work queue On Tue, 2014-04-08 at 09:55 -0700, Stephen Hemminger wrote: > On Mon, 07 Apr 2014 15:55:36 +0800 > Herbert Xu <herbert@...dor.apana.org.au> wrote: > > > +static void macvlan_broadcast_enqueue(struct macvlan_port *port, > > + struct sk_buff *skb) > > +{ > > + int err = -ENOMEM; > > + > > + skb = skb_clone(skb, GFP_ATOMIC); > > + if (unlikely(!skb)) > > + goto err; > > + > > + spin_lock(&port->bc_queue.lock); > > + if (skb_queue_len(&port->bc_queue) < skb->dev->tx_queue_len) { > > + __skb_queue_tail(&port->bc_queue, skb); > > + err = 0; > > + } > > + spin_unlock(&port->bc_queue.lock); > > + > > + if (unlikely(err)) { > > +err: > > + atomic_long_inc(&skb->dev->rx_dropped); > > + return; > > + } > > + > > unlikely() with goto is redundant and unnecessary. > > IMHO jumping into a block is bad code style > > Why not just move err: to the end as in: > > > skb = skb_clone(skb, GFP_ATOMIC); > if (unlikely(!skb)) > goto err; > > spin_lock(&port->bc_queue.lock); > if (skb_queue_len(&port->bc_queue) < skb->dev->tx_queue_len) { > __skb_queue_tail(&port->bc_queue, skb); > err = 0; > } > spin_unlock(&port->bc_queue.lock); > > if (err) > goto err; > > schedule_work(&port->bc_work); > return; > err: > > atomic_long_inc(&skb->dev->rx_dropped); > } Perhaps the spin_unlock should be duplicated instead and the "int err" removed. static void macvlan_broadcast_enqueue(struct macvlan_port *port, struct sk_buff *skb) { skb = skb_clone(skb, GFP_ATOMIC); if (!skb) goto err; spin_lock(&port->bc_queue.lock); if (skb_queue_len(&port->bc_queue) >= skb->dev->tx_queue_len) goto err_unlock; __skb_queue_tail(&port->bc_queue, skb); spin_unlock(&port->bc_queue.lock); schedule_work(&port->bc_work); return; err_unlock: spin_unlock(&port->bc_queue.lock); err: atomic_long_inc(&skb->dev->rx_dropped); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists