Messages in this thread Patch in this message | | | From | Ingo Oeser <> | 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 | Wed, 12 Apr 2006 21:32:44 +0200 |
| |
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/
| |