lkml.org 
[lkml]   [2006]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[RFD][PATCH] typhoon and core sample for folding away VLAN stuff (was: Re: [PATCH] deinline a few large functions in vlan code v2)
Date
Hi Denis,

here is a sample patch for the vlan core and API plus
typhoon driver converted as example.

Just so you can see, what I meant with "No #if in control flow code."

I couldn't resist cleaning up the vlan core, while I'm at it.
Of course I can seperate this, if you want the pure unilining stuff only.

So I hope this style is clear enough that you can do the rest of
the conversion without any problems.

Dave Dillow: Is this style clean enough to have it in your driver?

This kind of changes are important, because bloat creeps in byte by byte
of unused features. So I really appreciate your work here Denis.


Regards

Ingo Oeser

diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index c1ce87a..aab24b8 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -285,7 +285,9 @@ struct typhoon {
struct pci_dev * pdev;
struct net_device * dev;
spinlock_t state_lock;
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
struct vlan_group * vlgrp;
+#endif
struct basic_ring rxHiRing;
struct basic_ring rxBuffRing;
struct rxbuff_ent rxbuffers[RXENT_ENTRIES];
@@ -350,6 +352,19 @@ enum state_values {
#define TSO_OFFLOAD_ON 0
#endif

+
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+static inline struct vlan_group *typhoon_get_vlgrp(struct typhoon *tp)
+{
+ return tp->vlgrp;
+}
+#else
+static inline struct vlan_group *typhoon_get_vlgrp(struct typhoon *tp)
+{
+ return NULL;
+}
+#endif
+
static inline void
typhoon_inc_index(u32 *index, const int count, const int num_entries)
{
@@ -707,6 +722,7 @@ out:
return err;
}

+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
static void
typhoon_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)
{
@@ -754,6 +770,12 @@ typhoon_vlan_rx_kill_vid(struct net_devi
tp->vlgrp->vlan_devices[vid] = NULL;
spin_unlock_bh(&tp->state_lock);
}
+#else
+
+#define typhoon_vlan_rx_register NULL
+#define typhoon_vlan_rx_kill_vid NULL
+
+#endif

static inline void
typhoon_tso_fill(struct sk_buff *skb, struct transmit_ring *txRing,
@@ -1686,6 +1708,7 @@ typhoon_rx(struct typhoon *tp, struct ba
struct rx_desc *rx;
struct sk_buff *skb, *new_skb;
struct rxbuff_ent *rxb;
+ struct vlan_group *vlgrp;
dma_addr_t dma_addr;
u32 local_ready;
u32 rxaddr;
@@ -1745,8 +1768,9 @@ typhoon_rx(struct typhoon *tp, struct ba
new_skb->ip_summed = CHECKSUM_NONE;

spin_lock(&tp->state_lock);
- if(tp->vlgrp != NULL && rx->rxStatus & TYPHOON_RX_VLAN)
- vlan_hwaccel_receive_skb(new_skb, tp->vlgrp,
+ vlgrp = typhoon_get_vlgrp(tp);
+ if(vlgrp != NULL && rx->rxStatus & TYPHOON_RX_VLAN)
+ vlan_hwaccel_receive_skb(new_skb, vlgrp,
ntohl(rx->vlanTag) & 0xffff);
else
netif_receive_skb(new_skb);
@@ -2233,7 +2257,7 @@ typhoon_suspend(struct pci_dev *pdev, pm
return 0;

spin_lock_bh(&tp->state_lock);
- if(tp->vlgrp && tp->wol_events & TYPHOON_WAKE_MAGIC_PKT) {
+ if(typhoon_get_vlgrp(tp) && tp->wol_events & TYPHOON_WAKE_MAGIC_PKT) {
spin_unlock_bh(&tp->state_lock);
printk(KERN_ERR "%s: cannot do WAKE_MAGIC with VLANS\n",
dev->name);
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index eef0876..4ed541f 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -14,6 +14,8 @@
#define _LINUX_IF_VLAN_H_

#ifdef __KERNEL__
+#define HAVE_VLAN_PUT_TAG
+#define HAVE_VLAN_GET_TAG

/* externally defined structs */
struct vlan_group;
@@ -119,79 +121,29 @@ struct vlan_dev_info {
struct net_device_stats dev_stats; /* Device stats (rx-bytes, tx-pkts, etc...) */
};

-#define VLAN_DEV_INFO(x) ((struct vlan_dev_info *)(x->priv))
-
-/* inline functions */
-
-static inline struct net_device_stats *vlan_dev_get_stats(struct net_device *dev)
-{
- return &(VLAN_DEV_INFO(dev)->dev_stats);
-}
-
-static inline __u32 vlan_get_ingress_priority(struct net_device *dev,
- unsigned short vlan_tag)
-{
- struct vlan_dev_info *vip = VLAN_DEV_INFO(dev);
-
- return vip->ingress_priority_map[(vlan_tag >> 13) & 0x7];
-}
-
/* VLAN tx hw acceleration helpers. */
struct vlan_skb_tx_cookie {
u32 magic;
u32 vlan_tag;
};

-#define VLAN_TX_COOKIE_MAGIC 0x564c414e /* "VLAN" in ascii. */
#define VLAN_TX_SKB_CB(__skb) ((struct vlan_skb_tx_cookie *)&((__skb)->cb[0]))
-#define vlan_tx_tag_present(__skb) \
- (VLAN_TX_SKB_CB(__skb)->magic == VLAN_TX_COOKIE_MAGIC)
+
#define vlan_tx_tag_get(__skb) (VLAN_TX_SKB_CB(__skb)->vlan_tag)

-/* VLAN rx hw acceleration helper. This acts like netif_{rx,receive_skb}(). */
-static inline int __vlan_hwaccel_rx(struct sk_buff *skb,
- struct vlan_group *grp,
- unsigned short vlan_tag, int polling)
-{
- struct net_device_stats *stats;
-
- skb->dev = grp->vlan_devices[vlan_tag & VLAN_VID_MASK];
- if (skb->dev == NULL) {
- dev_kfree_skb_any(skb);
-
- /* Not NET_RX_DROP, this is not being dropped
- * due to congestion.
- */
- return 0;
- }
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+#define VLAN_DEV_INFO(x) ((struct vlan_dev_info *)(x->priv))
+#define VLAN_REAL_DEV(x) (VLAN_DEV_INFO(x)->real_dev)

- skb->dev->last_rx = jiffies;
+#define VLAN_TX_COOKIE_MAGIC 0x564c414e /* "VLAN" in ascii. */

- stats = vlan_dev_get_stats(skb->dev);
- stats->rx_packets++;
- stats->rx_bytes += skb->len;
-
- skb->priority = vlan_get_ingress_priority(skb->dev, vlan_tag);
- switch (skb->pkt_type) {
- case PACKET_BROADCAST:
- break;
-
- case PACKET_MULTICAST:
- stats->multicast++;
- break;
-
- case PACKET_OTHERHOST:
- /* Our lower layer thinks this is not local, let's make sure.
- * This allows the VLAN to have a different MAC than the underlying
- * device, and still route correctly.
- */
- if (!memcmp(eth_hdr(skb)->h_dest, skb->dev->dev_addr, ETH_ALEN))
- skb->pkt_type = PACKET_HOST;
- break;
- };

- return (polling ? netif_receive_skb(skb) : netif_rx(skb));
-}
+#define vlan_tx_tag_present(__skb) \
+ (VLAN_TX_SKB_CB(__skb)->magic == VLAN_TX_COOKIE_MAGIC)
+
+
+int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
+ unsigned short vlan_tag, int polling);

static inline int vlan_hwaccel_rx(struct sk_buff *skb,
struct vlan_group *grp,
@@ -207,55 +159,7 @@ static inline int vlan_hwaccel_receive_s
return __vlan_hwaccel_rx(skb, grp, vlan_tag, 1);
}

-/**
- * __vlan_put_tag - regular VLAN tag inserting
- * @skb: skbuff to tag
- * @tag: VLAN tag to insert
- *
- * Inserts the VLAN tag into @skb as part of the payload
- * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
- *
- * Following the skb_unshare() example, in case of error, the calling function
- * doesn't have to worry about freeing the original skb.
- */
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, unsigned short tag)
-{
- struct vlan_ethhdr *veth;
-
- if (skb_headroom(skb) < VLAN_HLEN) {
- struct sk_buff *sk_tmp = skb;
- skb = skb_realloc_headroom(sk_tmp, VLAN_HLEN);
- kfree_skb(sk_tmp);
- if (!skb) {
- printk(KERN_ERR "vlan: failed to realloc headroom\n");
- return NULL;
- }
- } else {
- skb = skb_unshare(skb, GFP_ATOMIC);
- if (!skb) {
- printk(KERN_ERR "vlan: failed to unshare skbuff\n");
- return NULL;
- }
- }
-
- veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
-
- /* Move the mac addresses to the beginning of the new header. */
- memmove(skb->data, skb->data + VLAN_HLEN, 2 * VLAN_ETH_ALEN);
-
- /* first, the ethernet type */
- veth->h_vlan_proto = __constant_htons(ETH_P_8021Q);
-
- /* now, the tag */
- veth->h_vlan_TCI = htons(tag);
-
- skb->protocol = __constant_htons(ETH_P_8021Q);
- skb->mac.raw -= VLAN_HLEN;
- skb->nh.raw -= VLAN_HLEN;
-
- return skb;
-}
-
+struct sk_buff *__vlan_put_tag(struct sk_buff *skb, unsigned short tag);
/**
* __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
* @skb: skbuff to tag
@@ -274,8 +178,6 @@ static inline struct sk_buff *__vlan_hwa
return skb;
}

-#define HAVE_VLAN_PUT_TAG
-
/**
* vlan_put_tag - inserts VLAN tag according to device features
* @skb: skbuff to tag
@@ -304,7 +206,7 @@ static inline int __vlan_get_tag(struct
{
struct vlan_ethhdr *veth = (struct vlan_ethhdr *)skb->data;

- if (veth->h_vlan_proto != __constant_htons(ETH_P_8021Q)) {
+ if (veth->h_vlan_proto != htons(ETH_P_8021Q)) {
return -EINVAL;
}

@@ -334,8 +236,6 @@ static inline int __vlan_hwaccel_get_tag
}
}

-#define HAVE_VLAN_GET_TAG
-
/**
* vlan_get_tag - get the VLAN ID from the skb
* @skb: skbuff to query
@@ -352,6 +252,41 @@ static inline int vlan_get_tag(struct sk
}
}

+#else
+
+#define VLAN_REAL_DEV(x) x
+#define vlan_tx_tag_present(__skb) 0
+
+static inline int vlan_hwaccel_rx(struct sk_buff *skb,
+ struct vlan_group *grp,
+ unsigned short vlan_tag)
+{
+ return netif_rx(skb);
+}
+
+static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
+ struct vlan_group *grp,
+ unsigned short vlan_tag)
+{
+ return netif_receive_skb(skb);
+}
+
+static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb, unsigned short tag)
+{
+ return skb;
+}
+
+static inline int vlan_get_tag(struct sk_buff *skb, unsigned short *tag)
+{
+ *tag = 0;
+ return -EINVAL;
+}
+
+#endif /* defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) */
+
+
+
+
#endif /* __KERNEL__ */

/* VLAN IOCTLs are found in sockios.h */
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index da9cfe9..d7c4eb1 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -37,6 +37,21 @@
#include <linux/if_vlan.h>
#include <net/ip.h>

+/* inline functions */
+
+static inline struct net_device_stats *vlan_dev_get_stats(struct net_device *dev)
+{
+ return &(VLAN_DEV_INFO(dev)->dev_stats);
+}
+
+static inline __u32 vlan_get_ingress_priority(struct net_device *dev,
+ unsigned short vlan_tag)
+{
+ struct vlan_dev_info *vip = VLAN_DEV_INFO(dev);
+
+ return vip->ingress_priority_map[(vlan_tag >> 13) & 0x7];
+}
+
/*
* Rebuild the Ethernet MAC header. This is called after an ARP
* (or in future other address resolution) has completed on this
@@ -71,7 +86,7 @@ int vlan_dev_rebuild_header(struct sk_bu
return 0;
}

-static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
+static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
{
if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
if (skb_shared(skb) || skb_cloned(skb)) {
@@ -90,6 +105,104 @@ static inline struct sk_buff *vlan_check
return skb;
}

+/**
+ * __vlan_put_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @tag: VLAN tag to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+struct sk_buff *__vlan_put_tag(struct sk_buff *skb, unsigned short tag)
+{
+ struct vlan_ethhdr *veth;
+
+ if (skb_headroom(skb) < VLAN_HLEN) {
+ struct sk_buff *sk_tmp = skb;
+ skb = skb_realloc_headroom(sk_tmp, VLAN_HLEN);
+ kfree_skb(sk_tmp);
+ if (!skb) {
+ printk(KERN_ERR "vlan: failed to realloc headroom\n");
+ return NULL;
+ }
+ } else {
+ skb = skb_unshare(skb, GFP_ATOMIC);
+ if (!skb) {
+ printk(KERN_ERR "vlan: failed to unshare skbuff\n");
+ return NULL;
+ }
+ }
+
+ veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
+
+ /* Move the mac addresses to the beginning of the new header. */
+ memmove(skb->data, skb->data + VLAN_HLEN, 2 * VLAN_ETH_ALEN);
+
+ /* first, the ethernet type */
+ veth->h_vlan_proto = __constant_htons(ETH_P_8021Q);
+
+ /* now, the tag */
+ veth->h_vlan_TCI = htons(tag);
+
+ skb->protocol = __constant_htons(ETH_P_8021Q);
+ skb->mac.raw -= VLAN_HLEN;
+ skb->nh.raw -= VLAN_HLEN;
+
+ return skb;
+}
+
+static void vlan_rx_common(struct sk_buff *skb, unsigned short vlan_tag)
+{
+ struct net_device_stats *stats;
+
+ stats = vlan_dev_get_stats(skb->dev);
+ stats->rx_packets++;
+ stats->rx_bytes += skb->len;
+
+ skb->priority = vlan_get_ingress_priority(skb->dev, vlan_tag);
+ switch (skb->pkt_type) {
+ case PACKET_BROADCAST:
+ break;
+
+ case PACKET_MULTICAST:
+ stats->multicast++;
+ break;
+
+ case PACKET_OTHERHOST:
+ /* Our lower layer thinks this is not local, let's make sure.
+ * This allows the VLAN to have a different MAC than the underlying
+ * device, and still route correctly.
+ */
+ if (!compare_ether_addr(eth_hdr(skb)->h_dest, skb->dev->dev_addr)) {
+ skb->pkt_type = PACKET_HOST;
+ break;
+ };
+}
+
+/* VLAN rx hw acceleration helper. This acts like netif_{rx,receive_skb}(). */
+int __vlan_hwaccel_rx(struct sk_buff *skb,
+ struct vlan_group *grp,
+ unsigned short vlan_tag, int polling)
+{
+ skb->dev = grp->vlan_devices[vlan_tag & VLAN_VID_MASK];
+ if (skb->dev == NULL) {
+ dev_kfree_skb_any(skb);
+
+ /* Not NET_RX_DROP, this is not being dropped
+ * due to congestion.
+ */
+ return 0;
+ }
+
+
+ skb->dev->last_rx = jiffies;
+ vlan_rx_common(skb, vlan_tag);
+ return (polling ? netif_receive_skb(skb) : netif_rx(skb));
+}
+
/*
* Determine the packet's protocol ID. The rule here is that we
* assume 802.3 if the type field is short enough to be a length.
@@ -158,11 +271,6 @@ int vlan_skb_recv(struct sk_buff *skb, s

skb->dev->last_rx = jiffies;

- /* Bump the rx counters for the VLAN device. */
- stats = vlan_dev_get_stats(skb->dev);
- stats->rx_packets++;
- stats->rx_bytes += skb->len;
-
/* Take off the VLAN header (4 bytes currently) */
skb_pull_rcsum(skb, VLAN_HLEN);

@@ -184,42 +292,7 @@ int vlan_skb_recv(struct sk_buff *skb, s
return -1;
}

- /*
- * Deal with ingress priority mapping.
- */
- skb->priority = vlan_get_ingress_priority(skb->dev, ntohs(vhdr->h_vlan_TCI));
-
-#ifdef VLAN_DEBUG
- printk(VLAN_DBG "%s: priority: %lu for TCI: %hu (hbo)\n",
- __FUNCTION__, (unsigned long)(skb->priority),
- ntohs(vhdr->h_vlan_TCI));
-#endif
-
- /* The ethernet driver already did the pkt_type calculations
- * for us...
- */
- switch (skb->pkt_type) {
- case PACKET_BROADCAST: /* Yeah, stats collect these together.. */
- // stats->broadcast ++; // no such counter :-(
- break;
-
- case PACKET_MULTICAST:
- stats->multicast++;
- break;
-
- case PACKET_OTHERHOST:
- /* Our lower layer thinks this is not local, let's make sure.
- * This allows the VLAN to have a different MAC than the underlying
- * device, and still route correctly.
- */
- if (!compare_ether_addr(eth_hdr(skb)->h_dest, skb->dev->dev_addr)) {
- /* It is for our (changed) MAC-address! */
- skb->pkt_type = PACKET_HOST;
- }
- break;
- default:
- break;
- };
+ vlan_rx_common(skb, vlan_tag);

/* Was a VLAN packet, grab the encapsulated protocol, which the layer
* three protocols care about.
@@ -258,7 +331,7 @@ int vlan_skb_recv(struct sk_buff *skb, s
* won't work for fault tolerant netware but does for the rest.
*/
if (*(unsigned short *)rawp == 0xFFFF) {
- skb->protocol = __constant_htons(ETH_P_802_3);
+ skb->protocol = htons(ETH_P_802_3);
/* place it back on the queue to be handled by true layer 3 protocols.
*/

@@ -281,7 +354,7 @@ int vlan_skb_recv(struct sk_buff *skb, s
/*
* Real 802.2 LLC
*/
- skb->protocol = __constant_htons(ETH_P_802_2);
+ skb->protocol = htons(ETH_P_802_2);
/* place it back on the queue to be handled by upper layer protocols.
*/

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 3da9264..dec3495 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -879,10 +879,8 @@ static unsigned int ip_sabotage_out(unsi
if ((out->hard_start_xmit == br_dev_xmit &&
okfn != br_nf_forward_finish &&
okfn != br_nf_local_out_finish && okfn != br_nf_dev_queue_xmit)
-#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
|| ((out->priv_flags & IFF_802_1Q_VLAN) &&
- VLAN_DEV_INFO(out)->real_dev->hard_start_xmit == br_dev_xmit)
-#endif
+ VLAN_REAL_DEV(out)->hard_start_xmit == br_dev_xmit)
) {
struct nf_bridge_info *nf_bridge;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-04-12 21:37    [from the cache]
©2003-2011 Jasper Spaans