[PATCH 1/5] net: pppoe - code cleanup and helpers

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/5] net: pppoe - code cleanup and helpers

Cyrill Gorcunov-2
- Introduce PPPOE_HASH_MASK.
- Remove redundant declaration of pppoe_chan_ops.
- Introduce stage_session helper.
- Tabs, space, long-line-split cleanup.

CC: Michal Ostrowski <[hidden email]>
Signed-off-by: Cyrill Gorcunov <[hidden email]>
---
 drivers/net/pppoe.c |  167 ++++++++++++++++++++++++++--------------------------
 1 file changed, 86 insertions(+), 81 deletions(-)

Index: linux-2.6.git/drivers/net/pppoe.c
===================================================================
--- linux-2.6.git.orig/drivers/net/pppoe.c
+++ linux-2.6.git/drivers/net/pppoe.c
@@ -84,32 +84,43 @@
 #include <asm/uaccess.h>
 
 #define PPPOE_HASH_BITS 4
-#define PPPOE_HASH_SIZE (1<<PPPOE_HASH_BITS)
-
-static struct ppp_channel_ops pppoe_chan_ops;
+#define PPPOE_HASH_SIZE (1 << PPPOE_HASH_BITS)
+#define PPPOE_HASH_MASK (PPPOE_HASH_SIZE - 1)
 
 static int pppoe_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
 static int pppoe_xmit(struct ppp_channel *chan, struct sk_buff *skb);
 static int __pppoe_xmit(struct sock *sk, struct sk_buff *skb);
 
 static const struct proto_ops pppoe_ops;
+static struct ppp_channel_ops pppoe_chan_ops;
 static DEFINE_RWLOCK(pppoe_hash_lock);
 
-static struct ppp_channel_ops pppoe_chan_ops;
+/*
+ * PPPoE could be in the following stages:
+ * 1) Discovery stage (to obtain remote MAC and Session ID)
+ * 2) Session stage (MAC and SID are known)
+ *
+ * Ethernet frames have a special tag for this but
+ * we use simplier approach based on session id
+ */
+static inline bool stage_session(__be16 sid)
+{
+ return sid != 0;
+}
 
 static inline int cmp_2_addr(struct pppoe_addr *a, struct pppoe_addr *b)
 {
- return (a->sid == b->sid &&
- (memcmp(a->remote, b->remote, ETH_ALEN) == 0));
+ return a->sid == b->sid &&
+ (memcmp(a->remote, b->remote, ETH_ALEN) == 0);
 }
 
 static inline int cmp_addr(struct pppoe_addr *a, __be16 sid, char *addr)
 {
- return (a->sid == sid &&
- (memcmp(a->remote,addr,ETH_ALEN) == 0));
+ return a->sid == sid &&
+ (memcmp(a->remote, addr, ETH_ALEN) == 0);
 }
 
-#if 8%PPPOE_HASH_BITS
+#if 8 % PPPOE_HASH_BITS
 #error 8 must be a multiple of PPPOE_HASH_BITS
 #endif
 
@@ -118,17 +129,14 @@ static int hash_item(__be16 sid, unsigne
  unsigned char hash = 0;
  unsigned int i;
 
- for (i = 0 ; i < ETH_ALEN ; i++) {
+ for (i = 0; i < ETH_ALEN; i++)
  hash ^= addr[i];
- }
- for (i = 0 ; i < sizeof(sid_t)*8 ; i += 8 ){
- hash ^= (__force __u32)sid>>i;
- }
- for (i = 8 ; (i>>=1) >= PPPOE_HASH_BITS ; ) {
- hash ^= hash>>i;
- }
+ for (i = 0; i < sizeof(sid_t) * 8; i += 8)
+ hash ^= (__force __u32)sid >> i;
+ for (i = 8; (i >>= 1) >= PPPOE_HASH_BITS;)
+ hash ^= hash >> i;
 
- return hash & ( PPPOE_HASH_SIZE - 1 );
+ return hash & PPPOE_HASH_MASK;
 }
 
 /* zeroed because its in .bss */
@@ -146,10 +154,15 @@ static struct pppox_sock *__get_item(__b
 
  ret = item_hash_table[hash];
 
- while (ret && !(cmp_addr(&ret->pppoe_pa, sid, addr) && ret->pppoe_ifindex == ifindex))
+ while (ret) {
+ if (cmp_addr(&ret->pppoe_pa, sid, addr) &&
+    ret->pppoe_ifindex == ifindex)
+ return ret;
+
  ret = ret->next;
+ }
 
- return ret;
+ return NULL;
 }
 
 static int __set_item(struct pppox_sock *po)
@@ -159,7 +172,8 @@ static int __set_item(struct pppox_sock
 
  ret = item_hash_table[hash];
  while (ret) {
- if (cmp_2_addr(&ret->pppoe_pa, &po->pppoe_pa) && ret->pppoe_ifindex == po->pppoe_ifindex)
+ if (cmp_2_addr(&ret->pppoe_pa, &po->pppoe_pa) &&
+    ret->pppoe_ifindex == po->pppoe_ifindex)
  return -EALREADY;
 
  ret = ret->next;
@@ -180,7 +194,8 @@ static struct pppox_sock *__delete_item(
  src = &item_hash_table[hash];
 
  while (ret) {
- if (cmp_addr(&ret->pppoe_pa, sid, addr) && ret->pppoe_ifindex == ifindex) {
+ if (cmp_addr(&ret->pppoe_pa, sid, addr) &&
+    ret->pppoe_ifindex == ifindex) {
  *src = ret->next;
  break;
  }
@@ -217,7 +232,7 @@ static inline struct pppox_sock *get_ite
  int ifindex;
 
  dev = dev_get_by_name(&init_net, sp->sa_addr.pppoe.dev);
- if(!dev)
+ if (!dev)
  return NULL;
  ifindex = dev->ifindex;
  dev_put(dev);
@@ -329,7 +344,6 @@ static struct notifier_block pppoe_notif
  .notifier_call = pppoe_device_event,
 };
 
-
 /************************************************************************
  *
  * Do the real work of receiving a PPPoE Session frame.
@@ -383,7 +397,8 @@ static int pppoe_rcv(struct sk_buff *skb
  struct pppox_sock *po;
  int len;
 
- if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!skb)
  goto out;
 
  if (dev_net(dev) != &init_net)
@@ -432,7 +447,8 @@ static int pppoe_disc_rcv(struct sk_buff
  if (dev_net(dev) != &init_net)
  goto abort;
 
- if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!skb)
  goto out;
 
  if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
@@ -493,12 +509,11 @@ static struct proto pppoe_sk_proto = {
  **********************************************************************/
 static int pppoe_create(struct net *net, struct socket *sock)
 {
- int error = -ENOMEM;
  struct sock *sk;
 
  sk = sk_alloc(net, PF_PPPOX, GFP_KERNEL, &pppoe_sk_proto);
  if (!sk)
- goto out;
+ return -ENOMEM;
 
  sock_init_data(sock, sk);
 
@@ -511,8 +526,7 @@ static int pppoe_create(struct net *net,
  sk->sk_family   = PF_PPPOX;
  sk->sk_protocol   = PX_PROTO_OE;
 
- error = 0;
-out: return error;
+ return 0;
 }
 
 static int pppoe_release(struct socket *sock)
@@ -524,7 +538,7 @@ static int pppoe_release(struct socket *
  return 0;
 
  lock_sock(sk);
- if (sock_flag(sk, SOCK_DEAD)){
+ if (sock_flag(sk, SOCK_DEAD)) {
  release_sock(sk);
  return -EBADF;
  }
@@ -542,7 +556,7 @@ static int pppoe_release(struct socket *
  write_lock_bh(&pppoe_hash_lock);
 
  po = pppox_sk(sk);
- if (po->pppoe_pa.sid) {
+ if (stage_session(po->pppoe_pa.sid)) {
  __delete_item(po->pppoe_pa.sid,
       po->pppoe_pa.remote, po->pppoe_ifindex);
  }
@@ -564,7 +578,6 @@ static int pppoe_release(struct socket *
  return 0;
 }
 
-
 static int pppoe_connect(struct socket *sock, struct sockaddr *uservaddr,
   int sockaddr_len, int flags)
 {
@@ -582,32 +595,31 @@ static int pppoe_connect(struct socket *
 
  /* Check for already bound sockets */
  error = -EBUSY;
- if ((sk->sk_state & PPPOX_CONNECTED) && sp->sa_addr.pppoe.sid)
+ if ((sk->sk_state & PPPOX_CONNECTED) &&
+     stage_session(sp->sa_addr.pppoe.sid))
  goto end;
 
  /* Check for already disconnected sockets, on attempts to disconnect */
  error = -EALREADY;
- if ((sk->sk_state & PPPOX_DEAD) && !sp->sa_addr.pppoe.sid )
+ if ((sk->sk_state & PPPOX_DEAD) &&
+     !stage_session(sp->sa_addr.pppoe.sid))
  goto end;
 
  error = 0;
- if (po->pppoe_pa.sid) {
- pppox_unbind_sock(sk);
-
- /* Delete the old binding */
- delete_item(po->pppoe_pa.sid,po->pppoe_pa.remote,po->pppoe_ifindex);
 
- if(po->pppoe_dev)
+ /* Delete the old binding */
+ if (stage_session(po->pppoe_pa.sid)) {
+ pppox_unbind_sock(sk);
+ delete_item(po->pppoe_pa.sid, po->pppoe_pa.remote, po->pppoe_ifindex);
+ if (po->pppoe_dev)
  dev_put(po->pppoe_dev);
-
  memset(sk_pppox(po) + 1, 0,
        sizeof(struct pppox_sock) - sizeof(struct sock));
-
  sk->sk_state = PPPOX_NONE;
  }
 
- /* Don't re-bind if sid==0 */
- if (sp->sa_addr.pppoe.sid != 0) {
+ /* Re-bind in session stage only */
+ if (stage_session(sp->sa_addr.pppoe.sid)) {
  dev = dev_get_by_name(&init_net, sp->sa_addr.pppoe.dev);
 
  error = -ENODEV;
@@ -618,7 +630,7 @@ static int pppoe_connect(struct socket *
  po->pppoe_ifindex = dev->ifindex;
 
  write_lock_bh(&pppoe_hash_lock);
- if (!(dev->flags & IFF_UP)){
+ if (!(dev->flags & IFF_UP)) {
  write_unlock_bh(&pppoe_hash_lock);
  goto err_put;
  }
@@ -648,7 +660,7 @@ static int pppoe_connect(struct socket *
 
  po->num = sp->sa_addr.pppoe.sid;
 
- end:
+end:
  release_sock(sk);
  return error;
 err_put:
@@ -659,7 +671,6 @@ err_put:
  goto end;
 }
 
-
 static int pppoe_getname(struct socket *sock, struct sockaddr *uaddr,
   int *usockaddr_len, int peer)
 {
@@ -678,7 +689,6 @@ static int pppoe_getname(struct socket *
  return 0;
 }
 
-
 static int pppoe_ioctl(struct socket *sock, unsigned int cmd,
  unsigned long arg)
 {
@@ -709,7 +719,7 @@ static int pppoe_ioctl(struct socket *so
  break;
 
  err = -EFAULT;
- if (get_user(val,(int __user *) arg))
+ if (get_user(val, (int __user *)arg))
  break;
 
  if (val < (po->pppoe_dev->mtu
@@ -722,7 +732,7 @@ static int pppoe_ioctl(struct socket *so
 
  case PPPIOCSFLAGS:
  err = -EFAULT;
- if (get_user(val, (int __user *) arg))
+ if (get_user(val, (int __user *)arg))
  break;
  err = 0;
  break;
@@ -749,7 +759,7 @@ static int pppoe_ioctl(struct socket *so
 
  err = -EINVAL;
  if (po->pppoe_relay.sa_family != AF_PPPOX ||
-    po->pppoe_relay.sa_protocol!= PX_PROTO_OE)
+    po->pppoe_relay.sa_protocol != PX_PROTO_OE)
  break;
 
  /* Check that the socket referenced by the address
@@ -781,7 +791,6 @@ static int pppoe_ioctl(struct socket *so
  return err;
 }
 
-
 static int pppoe_sendmsg(struct kiocb *iocb, struct socket *sock,
   struct msghdr *m, size_t total_len)
 {
@@ -808,7 +817,7 @@ static int pppoe_sendmsg(struct kiocb *i
  dev = po->pppoe_dev;
 
  error = -EMSGSIZE;
- if (total_len > (dev->mtu + dev->hard_header_len))
+ if (total_len > (dev->mtu + dev->hard_header_len))
  goto end;
 
 
@@ -853,7 +862,6 @@ end:
  return error;
 }
 
-
 /************************************************************************
  *
  * xmit function for internal use.
@@ -903,7 +911,6 @@ abort:
  return 1;
 }
 
-
 /************************************************************************
  *
  * xmit function called by generic PPP driver
@@ -916,7 +923,6 @@ static int pppoe_xmit(struct ppp_channel
  return __pppoe_xmit(sk, skb);
 }
 
-
 static struct ppp_channel_ops pppoe_chan_ops = {
  .start_xmit = pppoe_xmit,
 };
@@ -976,9 +982,9 @@ out:
 static __inline__ struct pppox_sock *pppoe_get_idx(loff_t pos)
 {
  struct pppox_sock *po;
- int i = 0;
+ int i;
 
- for (; i < PPPOE_HASH_SIZE; i++) {
+ for (i = 0; i < PPPOE_HASH_SIZE; i++) {
  po = item_hash_table[i];
  while (po) {
  if (!pos--)
@@ -1064,32 +1070,31 @@ static inline int pppoe_proc_init(void)
 #endif /* CONFIG_PROC_FS */
 
 static const struct proto_ops pppoe_ops = {
-    .family = AF_PPPOX,
-    .owner = THIS_MODULE,
-    .release = pppoe_release,
-    .bind = sock_no_bind,
-    .connect = pppoe_connect,
-    .socketpair = sock_no_socketpair,
-    .accept = sock_no_accept,
-    .getname = pppoe_getname,
-    .poll = datagram_poll,
-    .listen = sock_no_listen,
-    .shutdown = sock_no_shutdown,
-    .setsockopt = sock_no_setsockopt,
-    .getsockopt = sock_no_getsockopt,
-    .sendmsg = pppoe_sendmsg,
-    .recvmsg = pppoe_recvmsg,
-    .mmap = sock_no_mmap,
-    .ioctl = pppox_ioctl,
+ .family = AF_PPPOX,
+ .owner = THIS_MODULE,
+ .release = pppoe_release,
+ .bind = sock_no_bind,
+ .connect = pppoe_connect,
+ .socketpair = sock_no_socketpair,
+ .accept = sock_no_accept,
+ .getname = pppoe_getname,
+ .poll = datagram_poll,
+ .listen = sock_no_listen,
+ .shutdown = sock_no_shutdown,
+ .setsockopt = sock_no_setsockopt,
+ .getsockopt = sock_no_getsockopt,
+ .sendmsg = pppoe_sendmsg,
+ .recvmsg = pppoe_recvmsg,
+ .mmap = sock_no_mmap,
+ .ioctl = pppox_ioctl,
 };
 
 static struct pppox_proto pppoe_proto = {
-    .create = pppoe_create,
-    .ioctl = pppoe_ioctl,
-    .owner = THIS_MODULE,
+ .create = pppoe_create,
+ .ioctl = pppoe_ioctl,
+ .owner = THIS_MODULE,
 };
 
-
 static int __init pppoe_init(void)
 {
  int err = proto_register(&pppoe_sk_proto, 0);
@@ -1097,7 +1102,7 @@ static int __init pppoe_init(void)
  if (err)
  goto out;
 
- err = register_pppox_proto(PX_PROTO_OE, &pppoe_proto);
+ err = register_pppox_proto(PX_PROTO_OE, &pppoe_proto);
  if (err)
  goto out_unregister_pppoe_proto;
 

--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Loading...