Patchwork [1/6] vxlan: minor output refactoring

login
register
mail settings
Submitter stephen hemminger
Date Oct. 9, 2012, 5:56 p.m.
Message ID <20121009175714.462171246@vyatta.com>
Download mbox | patch
Permalink /patch/190398/
State Superseded
Delegated to: David Miller
Headers show

Comments

stephen hemminger - Oct. 9, 2012, 5:56 p.m.
Move code to find destination to a small function.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches - Oct. 9, 2012, 6:27 p.m.
On Tue, 2012-10-09 at 10:56 -0700, Stephen Hemminger wrote:
> +static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> +{
> +       const struct ethhdr *eth = (struct ethhdr *) skb->data;
> +       struct vxlan_fdb *f;
> +
> +       if (is_multicast_ether_addr(eth->h_dest) ||
> +           (f = vxlan_find_mac(vxlan, eth->h_dest)) == NULL)
> +               return vxlan->gaddr;
> +       else
> +               return f->remote_ip;
> +}

Bikeshedding:

This might be simpler to read with a few more lines like:

static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
{
	const struct ethhdr *eth = (const struct ethhdr *)skb->data;
	struct vxlan_fdb *f;

	if (is_multicast_ether_addr(eth->h_dest))
		return vxlan->gaddr;

	f = vxlan_find_mac(vxlan, eth->h_dest);
	if (!f)
		return vxlan->gaddr;

	return f->remote_ip;
}

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger - Oct. 9, 2012, 6:29 p.m.
On Tue, 09 Oct 2012 11:27:55 -0700
Joe Perches <joe@perches.com> wrote:

> On Tue, 2012-10-09 at 10:56 -0700, Stephen Hemminger wrote:
> > +static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> > +{
> > +       const struct ethhdr *eth = (struct ethhdr *) skb->data;
> > +       struct vxlan_fdb *f;
> > +
> > +       if (is_multicast_ether_addr(eth->h_dest) ||
> > +           (f = vxlan_find_mac(vxlan, eth->h_dest)) == NULL)
> > +               return vxlan->gaddr;
> > +       else
> > +               return f->remote_ip;
> > +}
> 
> Bikeshedding:
> 
> This might be simpler to read with a few more lines like:
> 
> static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> {
> 	const struct ethhdr *eth = (const struct ethhdr *)skb->data;
> 	struct vxlan_fdb *f;
> 
> 	if (is_multicast_ether_addr(eth->h_dest))
> 		return vxlan->gaddr;
> 
> 	f = vxlan_find_mac(vxlan, eth->h_dest);
> 	if (!f)
> 		return vxlan->gaddr;
> 
> 	return f->remote_ip;
> }
> 

Maybe. Doesn't really matter that much.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight - Oct. 10, 2012, 8:08 a.m.
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Joe Perches
> Sent: 09 October 2012 19:28
> To: Stephen Hemminger
> Cc: David Miller; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/6] vxlan: minor output refactoring
> 
> On Tue, 2012-10-09 at 10:56 -0700, Stephen Hemminger wrote:
> > +static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> > +{
> > +       const struct ethhdr *eth = (struct ethhdr *) skb->data;
> > +       struct vxlan_fdb *f;
> > +
> > +       if (is_multicast_ether_addr(eth->h_dest) ||
> > +           (f = vxlan_find_mac(vxlan, eth->h_dest)) == NULL)
> > +               return vxlan->gaddr;
> > +       else
> > +               return f->remote_ip;
> > +}
> 
> Bikeshedding:
> 
> This might be simpler to read with a few more lines like:
> 
> static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
> {
> 	const struct ethhdr *eth = (const struct ethhdr *)skb->data;
> 	struct vxlan_fdb *f;
> 
> 	if (is_multicast_ether_addr(eth->h_dest))
> 		return vxlan->gaddr;
> 
> 	f = vxlan_find_mac(vxlan, eth->h_dest);
> 	if (!f)
> 		return vxlan->gaddr;
> 
> 	return f->remote_ip;
> }

or, painting it green:
static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
{
       const struct ethhdr *eth = (struct ethhdr *)skb->data;
       struct vxlan_fdb *f;

       if (!is_multicast_ether_addr(eth->h_dest) &&
           (f = vxlan_find_mac(vxlan, eth->h_dest)) != NULL)
               return f->remote_ip;
       return vxlan->gaddr;
}

	David



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

Patch

--- a/drivers/net/vxlan.c	2012-10-09 10:34:57.000000000 -0700
+++ b/drivers/net/vxlan.c	2012-10-09 10:48:56.546879617 -0700
@@ -621,6 +621,18 @@  static inline u8 vxlan_ecn_encap(u8 tos,
 	return INET_ECN_encapsulate(tos, inner);
 }
 
+static __be32 vxlan_find_dst(struct vxlan_dev *vxlan, struct sk_buff *skb)
+{
+	const struct ethhdr *eth = (struct ethhdr *) skb->data;
+	struct vxlan_fdb *f;
+
+	if (is_multicast_ether_addr(eth->h_dest) ||
+	    (f = vxlan_find_mac(vxlan, eth->h_dest)) == NULL)
+		return vxlan->gaddr;
+	else
+		return f->remote_ip;
+}
+
 /* Transmit local packets over Vxlan
  *
  * Outer IP header inherits ECN and DF from inner header.
@@ -632,13 +644,11 @@  static netdev_tx_t vxlan_xmit(struct sk_
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	struct rtable *rt;
-	const struct ethhdr *eth;
 	const struct iphdr *old_iph;
 	struct iphdr *iph;
 	struct vxlanhdr *vxh;
 	struct udphdr *uh;
 	struct flowi4 fl4;
-	struct vxlan_fdb *f;
 	unsigned int pkt_len = skb->len;
 	u32 hash;
 	__be32 dst;
@@ -646,21 +656,16 @@  static netdev_tx_t vxlan_xmit(struct sk_
 	__u8 tos, ttl;
 	int err;
 
+	dst = vxlan_find_dst(vxlan, skb);
+	if (!dst)
+		goto drop;
+
 	/* Need space for new headers (invalidates iph ptr) */
 	if (skb_cow_head(skb, VXLAN_HEADROOM))
 		goto drop;
 
-	eth = (void *)skb->data;
 	old_iph = ip_hdr(skb);
 
-	if (!is_multicast_ether_addr(eth->h_dest) &&
-	    (f = vxlan_find_mac(vxlan, eth->h_dest)))
-		dst = f->remote_ip;
-	else if (vxlan->gaddr) {
-		dst = vxlan->gaddr;
-	} else
-		goto drop;
-
 	ttl = vxlan->ttl;
 	if (!ttl && IN_MULTICAST(ntohl(dst)))
 		ttl = 1;