[PATCH] decouple llc/bridge

[PATCH] decouple llc/bridge

[Dave Jones]

I was wondering why the llc code was getting compiled and it turned out
to be because I had bridging enabled. It turns out to only needs it for
a single function (llc_mac_hdr_init).

Converting this to a static inline like the other llc functions it uses
allows to decouple the dependency

Signed-off-by: Dave Jones <davej@codemonkey.org.uk>

diff --git include/net/llc.h include/net/llc.h
index e250dca03963..edcb120ee6b0 100644
--- include/net/llc.h
+++ include/net/llc.h
@@ -13,6 +13,7 @@
  */
 
 #include <linux/if.h>
+#include <linux/if_arp.h>
 #include <linux/if_ether.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
@@ -100,8 +101,34 @@ extern struct list_head llc_sap_list;
 int llc_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 	    struct net_device *orig_dev);
 
-int llc_mac_hdr_init(struct sk_buff *skb, const unsigned char *sa,
-		     const unsigned char *da);
+/**
+ *      llc_mac_hdr_init - fills MAC header fields
+ *      @skb: Address of the frame to initialize its MAC header
+ *      @sa: The MAC source address
+ *      @da: The MAC destination address
+ *
+ *      Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
+ *      is a valid type and initialization completes correctly 1, otherwise.
+ */
+static inline int llc_mac_hdr_init(struct sk_buff *skb,
+				   const unsigned char *sa, const unsigned char *da)
+{
+	int rc = -EINVAL;
+
+	switch (skb->dev->type) {
+	case ARPHRD_ETHER:
+	case ARPHRD_LOOPBACK:
+		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
+				     skb->len);
+		if (rc > 0)
+			rc = 0;
+		break;
+	default:
+		break;
+	}
+	return rc;
+}
+
 
 void llc_add_pack(int type,
 		  void (*handler)(struct llc_sap *sap, struct sk_buff *skb));
diff --git net/802/Kconfig net/802/Kconfig
index aaa83e888240..8bea5d1d5118 100644
--- net/802/Kconfig
+++ net/802/Kconfig
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config STP
 	tristate
-	select LLC
 
 config GARP
 	tristate
diff --git net/bridge/Kconfig net/bridge/Kconfig
index 3c8ded7d3e84..c011856d3386 100644
--- net/bridge/Kconfig
+++ net/bridge/Kconfig
@@ -5,7 +5,6 @@
 
 config BRIDGE
 	tristate "802.1d Ethernet Bridging"
-	select LLC
 	select STP
 	depends on IPV6 || IPV6=n
 	help
diff --git net/llc/llc_output.c net/llc/llc_output.c
index 5a6466fc626a..ad66886ed141 100644
--- net/llc/llc_output.c
+++ net/llc/llc_output.c
@@ -13,34 +13,6 @@
 #include <net/llc.h>
 #include <net/llc_pdu.h>
 
-/**
- *	llc_mac_hdr_init - fills MAC header fields
- *	@skb: Address of the frame to initialize its MAC header
- *	@sa: The MAC source address
- *	@da: The MAC destination address
- *
- *	Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
- *	is a valid type and initialization completes correctly 1, otherwise.
- */
-int llc_mac_hdr_init(struct sk_buff *skb,
-		     const unsigned char *sa, const unsigned char *da)
-{
-	int rc = -EINVAL;
-
-	switch (skb->dev->type) {
-	case ARPHRD_ETHER:
-	case ARPHRD_LOOPBACK:
-		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
-				     skb->len);
-		if (rc > 0)
-			rc = 0;
-		break;
-	default:
-		break;
-	}
-	return rc;
-}
-
 /**
  *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
  *	@sap: sap to use

[Bridge] [PATCH] decouple llc/bridge

[Dave Jones]

I was wondering why the llc code was getting compiled and it turned out
to be because I had bridging enabled. It turns out to only needs it for
a single function (llc_mac_hdr_init).

Converting this to a static inline like the other llc functions it uses
allows to decouple the dependency

Signed-off-by: Dave Jones <davej@codemonkey.org.uk>

diff --git include/net/llc.h include/net/llc.h
index e250dca03963..edcb120ee6b0 100644
--- include/net/llc.h
+++ include/net/llc.h
@@ -13,6 +13,7 @@
  */
 
 #include <linux/if.h>
+#include <linux/if_arp.h>
 #include <linux/if_ether.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
@@ -100,8 +101,34 @@ extern struct list_head llc_sap_list;
 int llc_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 	    struct net_device *orig_dev);
 
-int llc_mac_hdr_init(struct sk_buff *skb, const unsigned char *sa,
-		     const unsigned char *da);
+/**
+ *      llc_mac_hdr_init - fills MAC header fields
+ *      @skb: Address of the frame to initialize its MAC header
+ *      @sa: The MAC source address
+ *      @da: The MAC destination address
+ *
+ *      Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
+ *      is a valid type and initialization completes correctly 1, otherwise.
+ */
+static inline int llc_mac_hdr_init(struct sk_buff *skb,
+				   const unsigned char *sa, const unsigned char *da)
+{
+	int rc = -EINVAL;
+
+	switch (skb->dev->type) {
+	case ARPHRD_ETHER:
+	case ARPHRD_LOOPBACK:
+		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
+				     skb->len);
+		if (rc > 0)
+			rc = 0;
+		break;
+	default:
+		break;
+	}
+	return rc;
+}
+
 
 void llc_add_pack(int type,
 		  void (*handler)(struct llc_sap *sap, struct sk_buff *skb));
diff --git net/802/Kconfig net/802/Kconfig
index aaa83e888240..8bea5d1d5118 100644
--- net/802/Kconfig
+++ net/802/Kconfig
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config STP
 	tristate
-	select LLC
 
 config GARP
 	tristate
diff --git net/bridge/Kconfig net/bridge/Kconfig
index 3c8ded7d3e84..c011856d3386 100644
--- net/bridge/Kconfig
+++ net/bridge/Kconfig
@@ -5,7 +5,6 @@
 
 config BRIDGE
 	tristate "802.1d Ethernet Bridging"
-	select LLC
 	select STP
 	depends on IPV6 || IPV6=n
 	help
diff --git net/llc/llc_output.c net/llc/llc_output.c
index 5a6466fc626a..ad66886ed141 100644
--- net/llc/llc_output.c
+++ net/llc/llc_output.c
@@ -13,34 +13,6 @@
 #include <net/llc.h>
 #include <net/llc_pdu.h>
 
-/**
- *	llc_mac_hdr_init - fills MAC header fields
- *	@skb: Address of the frame to initialize its MAC header
- *	@sa: The MAC source address
- *	@da: The MAC destination address
- *
- *	Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
- *	is a valid type and initialization completes correctly 1, otherwise.
- */
-int llc_mac_hdr_init(struct sk_buff *skb,
-		     const unsigned char *sa, const unsigned char *da)
-{
-	int rc = -EINVAL;
-
-	switch (skb->dev->type) {
-	case ARPHRD_ETHER:
-	case ARPHRD_LOOPBACK:
-		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
-				     skb->len);
-		if (rc > 0)
-			rc = 0;
-		break;
-	default:
-		break;
-	}
-	return rc;
-}
-
 /**
  *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
  *	@sap: sap to use

Re: [PATCH] decouple llc/bridge

[Stephen Hemminger]

On Thu, 7 Apr 2022 11:12:17 -0400
Dave Jones <davej@codemonkey.org.uk> wrote:

> I was wondering why the llc code was getting compiled and it turned out
> to be because I had bridging enabled. It turns out to only needs it for
> a single function (llc_mac_hdr_init).
> 
> Converting this to a static inline like the other llc functions it uses
> allows to decouple the dependency
> 
> Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
> 
> diff --git include/net/llc.h include/net/llc.h
> index e250dca03963..edcb120ee6b0 100644
> --- include/net/llc.h
> +++ include/net/llc.h
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/if.h>
> +#include <linux/if_arp.h>
>  #include <linux/if_ether.h>
>  #include <linux/list.h>
>  #include <linux/spinlock.h>
> @@ -100,8 +101,34 @@ extern struct list_head llc_sap_list;
>  int llc_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
>  	    struct net_device *orig_dev);
>  
> -int llc_mac_hdr_init(struct sk_buff *skb, const unsigned char *sa,
> -		     const unsigned char *da);
> +/**
> + *      llc_mac_hdr_init - fills MAC header fields
> + *      @skb: Address of the frame to initialize its MAC header
> + *      @sa: The MAC source address
> + *      @da: The MAC destination address
> + *
> + *      Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
> + *      is a valid type and initialization completes correctly 1, otherwise.
> + */
> +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> +				   const unsigned char *sa, const unsigned char *da)
> +{
> +	int rc = -EINVAL;
> +
> +	switch (skb->dev->type) {
> +	case ARPHRD_ETHER:
> +	case ARPHRD_LOOPBACK:
> +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> +				     skb->len);
> +		if (rc > 0)
> +			rc = 0;
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  
>  void llc_add_pack(int type,
>  		  void (*handler)(struct llc_sap *sap, struct sk_buff *skb));
> diff --git net/802/Kconfig net/802/Kconfig
> index aaa83e888240..8bea5d1d5118 100644
> --- net/802/Kconfig
> +++ net/802/Kconfig
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config STP
>  	tristate
> -	select LLC
>  
>  config GARP
>  	tristate
> diff --git net/bridge/Kconfig net/bridge/Kconfig
> index 3c8ded7d3e84..c011856d3386 100644
> --- net/bridge/Kconfig
> +++ net/bridge/Kconfig
> @@ -5,7 +5,6 @@
>  
>  config BRIDGE
>  	tristate "802.1d Ethernet Bridging"
> -	select LLC
>  	select STP
>  	depends on IPV6 || IPV6=n
>  	help
> diff --git net/llc/llc_output.c net/llc/llc_output.c
> index 5a6466fc626a..ad66886ed141 100644
> --- net/llc/llc_output.c
> +++ net/llc/llc_output.c
> @@ -13,34 +13,6 @@
>  #include <net/llc.h>
>  #include <net/llc_pdu.h>
>  
> -/**
> - *	llc_mac_hdr_init - fills MAC header fields
> - *	@skb: Address of the frame to initialize its MAC header
> - *	@sa: The MAC source address
> - *	@da: The MAC destination address
> - *
> - *	Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
> - *	is a valid type and initialization completes correctly 1, otherwise.
> - */
> -int llc_mac_hdr_init(struct sk_buff *skb,
> -		     const unsigned char *sa, const unsigned char *da)
> -{
> -	int rc = -EINVAL;
> -
> -	switch (skb->dev->type) {
> -	case ARPHRD_ETHER:
> -	case ARPHRD_LOOPBACK:
> -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> -				     skb->len);
> -		if (rc > 0)
> -			rc = 0;
> -		break;
> -	default:
> -		break;
> -	}
> -	return rc;
> -}
> -
>  /**
>   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
>   *	@sap: sap to use

You may break other uses of LLC.

Why not open code as different function.  I used the llc stuff because there
were multiple copies of same code (DRY).

Re: [Bridge] [PATCH] decouple llc/bridge

[Stephen Hemminger]

On Thu, 7 Apr 2022 11:12:17 -0400
Dave Jones <davej@codemonkey.org.uk> wrote:

> I was wondering why the llc code was getting compiled and it turned out
> to be because I had bridging enabled. It turns out to only needs it for
> a single function (llc_mac_hdr_init).
> 
> Converting this to a static inline like the other llc functions it uses
> allows to decouple the dependency
> 
> Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
> 
> diff --git include/net/llc.h include/net/llc.h
> index e250dca03963..edcb120ee6b0 100644
> --- include/net/llc.h
> +++ include/net/llc.h
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/if.h>
> +#include <linux/if_arp.h>
>  #include <linux/if_ether.h>
>  #include <linux/list.h>
>  #include <linux/spinlock.h>
> @@ -100,8 +101,34 @@ extern struct list_head llc_sap_list;
>  int llc_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
>  	    struct net_device *orig_dev);
>  
> -int llc_mac_hdr_init(struct sk_buff *skb, const unsigned char *sa,
> -		     const unsigned char *da);
> +/**
> + *      llc_mac_hdr_init - fills MAC header fields
> + *      @skb: Address of the frame to initialize its MAC header
> + *      @sa: The MAC source address
> + *      @da: The MAC destination address
> + *
> + *      Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
> + *      is a valid type and initialization completes correctly 1, otherwise.
> + */
> +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> +				   const unsigned char *sa, const unsigned char *da)
> +{
> +	int rc = -EINVAL;
> +
> +	switch (skb->dev->type) {
> +	case ARPHRD_ETHER:
> +	case ARPHRD_LOOPBACK:
> +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> +				     skb->len);
> +		if (rc > 0)
> +			rc = 0;
> +		break;
> +	default:
> +		break;
> +	}
> +	return rc;
> +}
> +
>  
>  void llc_add_pack(int type,
>  		  void (*handler)(struct llc_sap *sap, struct sk_buff *skb));
> diff --git net/802/Kconfig net/802/Kconfig
> index aaa83e888240..8bea5d1d5118 100644
> --- net/802/Kconfig
> +++ net/802/Kconfig
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config STP
>  	tristate
> -	select LLC
>  
>  config GARP
>  	tristate
> diff --git net/bridge/Kconfig net/bridge/Kconfig
> index 3c8ded7d3e84..c011856d3386 100644
> --- net/bridge/Kconfig
> +++ net/bridge/Kconfig
> @@ -5,7 +5,6 @@
>  
>  config BRIDGE
>  	tristate "802.1d Ethernet Bridging"
> -	select LLC
>  	select STP
>  	depends on IPV6 || IPV6=n
>  	help
> diff --git net/llc/llc_output.c net/llc/llc_output.c
> index 5a6466fc626a..ad66886ed141 100644
> --- net/llc/llc_output.c
> +++ net/llc/llc_output.c
> @@ -13,34 +13,6 @@
>  #include <net/llc.h>
>  #include <net/llc_pdu.h>
>  
> -/**
> - *	llc_mac_hdr_init - fills MAC header fields
> - *	@skb: Address of the frame to initialize its MAC header
> - *	@sa: The MAC source address
> - *	@da: The MAC destination address
> - *
> - *	Fills MAC header fields, depending on MAC type. Returns 0, If MAC type
> - *	is a valid type and initialization completes correctly 1, otherwise.
> - */
> -int llc_mac_hdr_init(struct sk_buff *skb,
> -		     const unsigned char *sa, const unsigned char *da)
> -{
> -	int rc = -EINVAL;
> -
> -	switch (skb->dev->type) {
> -	case ARPHRD_ETHER:
> -	case ARPHRD_LOOPBACK:
> -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> -				     skb->len);
> -		if (rc > 0)
> -			rc = 0;
> -		break;
> -	default:
> -		break;
> -	}
> -	return rc;
> -}
> -
>  /**
>   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
>   *	@sap: sap to use

You may break other uses of LLC.

Why not open code as different function.  I used the llc stuff because there
were multiple copies of same code (DRY).

Re: [PATCH] decouple llc/bridge

[Jakub Kicinski]

On Thu, 7 Apr 2022 09:16:40 -0700 Stephen Hemminger wrote:
> > I was wondering why the llc code was getting compiled and it turned out
> > to be because I had bridging enabled. It turns out to only needs it for
> > a single function (llc_mac_hdr_init).

> > +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> > +				   const unsigned char *sa, const unsigned char *da)
> > +{
> > +	int rc = -EINVAL;
> > +
> > +	switch (skb->dev->type) {
> > +	case ARPHRD_ETHER:
> > +	case ARPHRD_LOOPBACK:
> > +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > +				     skb->len);
> > +		if (rc > 0)
> > +			rc = 0;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +	return rc;
> > +}
> > +
> >  

nit: extra new line

> > -int llc_mac_hdr_init(struct sk_buff *skb,
> > -		     const unsigned char *sa, const unsigned char *da)
> > -{
> > -	int rc = -EINVAL;
> > -
> > -	switch (skb->dev->type) {
> > -	case ARPHRD_ETHER:
> > -	case ARPHRD_LOOPBACK:
> > -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > -				     skb->len);
> > -		if (rc > 0)
> > -			rc = 0;
> > -		break;
> > -	default:
> > -		break;
> > -	}
> > -	return rc;
> > -}

There's also an EXPORT somewhere in this file that has to go.

> >  /**
> >   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
> >   *	@sap: sap to use  
> 
> You may break other uses of LLC.
> 
> Why not open code as different function.  I used the llc stuff because there
> were multiple copies of same code (DRY).

I didn't quite get what you mean, Stephen, would you mind restating?

Re: [Bridge] [PATCH] decouple llc/bridge

[Jakub Kicinski]

On Thu, 7 Apr 2022 09:16:40 -0700 Stephen Hemminger wrote:
> > I was wondering why the llc code was getting compiled and it turned out
> > to be because I had bridging enabled. It turns out to only needs it for
> > a single function (llc_mac_hdr_init).

> > +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> > +				   const unsigned char *sa, const unsigned char *da)
> > +{
> > +	int rc = -EINVAL;
> > +
> > +	switch (skb->dev->type) {
> > +	case ARPHRD_ETHER:
> > +	case ARPHRD_LOOPBACK:
> > +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > +				     skb->len);
> > +		if (rc > 0)
> > +			rc = 0;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +	return rc;
> > +}
> > +
> >  

nit: extra new line

> > -int llc_mac_hdr_init(struct sk_buff *skb,
> > -		     const unsigned char *sa, const unsigned char *da)
> > -{
> > -	int rc = -EINVAL;
> > -
> > -	switch (skb->dev->type) {
> > -	case ARPHRD_ETHER:
> > -	case ARPHRD_LOOPBACK:
> > -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > -				     skb->len);
> > -		if (rc > 0)
> > -			rc = 0;
> > -		break;
> > -	default:
> > -		break;
> > -	}
> > -	return rc;
> > -}

There's also an EXPORT somewhere in this file that has to go.

> >  /**
> >   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
> >   *	@sap: sap to use  
> 
> You may break other uses of LLC.
> 
> Why not open code as different function.  I used the llc stuff because there
> were multiple copies of same code (DRY).

I didn't quite get what you mean, Stephen, would you mind restating?

Re: [PATCH] decouple llc/bridge

[Stephen Hemminger]

On Thu, 7 Apr 2022 19:48:59 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 7 Apr 2022 09:16:40 -0700 Stephen Hemminger wrote:
> > > I was wondering why the llc code was getting compiled and it turned out
> > > to be because I had bridging enabled. It turns out to only needs it for
> > > a single function (llc_mac_hdr_init).  
> 
> > > +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> > > +				   const unsigned char *sa, const unsigned char *da)
> > > +{
> > > +	int rc = -EINVAL;
> > > +
> > > +	switch (skb->dev->type) {
> > > +	case ARPHRD_ETHER:
> > > +	case ARPHRD_LOOPBACK:
> > > +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > +				     skb->len);
> > > +		if (rc > 0)
> > > +			rc = 0;
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > > +	return rc;
> > > +}
> > > +
> > >    
> 
> nit: extra new line
> 
> > > -int llc_mac_hdr_init(struct sk_buff *skb,
> > > -		     const unsigned char *sa, const unsigned char *da)
> > > -{
> > > -	int rc = -EINVAL;
> > > -
> > > -	switch (skb->dev->type) {
> > > -	case ARPHRD_ETHER:
> > > -	case ARPHRD_LOOPBACK:
> > > -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > -				     skb->len);
> > > -		if (rc > 0)
> > > -			rc = 0;
> > > -		break;
> > > -	default:
> > > -		break;
> > > -	}
> > > -	return rc;
> > > -}  
> 
> There's also an EXPORT somewhere in this file that has to go.
> 
> > >  /**
> > >   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
> > >   *	@sap: sap to use    
> > 
> > You may break other uses of LLC.
> > 
> > Why not open code as different function.  I used the llc stuff because there
> > were multiple copies of same code (DRY).  
> 
> I didn't quite get what you mean, Stephen, would you mind restating?


Short answer: it was idea that didn't pan out.

Suggestion: get rid of using LLC  in bridge and just rewrite that one place.

Re: [Bridge] [PATCH] decouple llc/bridge

[Stephen Hemminger]

On Thu, 7 Apr 2022 19:48:59 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 7 Apr 2022 09:16:40 -0700 Stephen Hemminger wrote:
> > > I was wondering why the llc code was getting compiled and it turned out
> > > to be because I had bridging enabled. It turns out to only needs it for
> > > a single function (llc_mac_hdr_init).  
> 
> > > +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> > > +				   const unsigned char *sa, const unsigned char *da)
> > > +{
> > > +	int rc = -EINVAL;
> > > +
> > > +	switch (skb->dev->type) {
> > > +	case ARPHRD_ETHER:
> > > +	case ARPHRD_LOOPBACK:
> > > +		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > +				     skb->len);
> > > +		if (rc > 0)
> > > +			rc = 0;
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > > +	return rc;
> > > +}
> > > +
> > >    
> 
> nit: extra new line
> 
> > > -int llc_mac_hdr_init(struct sk_buff *skb,
> > > -		     const unsigned char *sa, const unsigned char *da)
> > > -{
> > > -	int rc = -EINVAL;
> > > -
> > > -	switch (skb->dev->type) {
> > > -	case ARPHRD_ETHER:
> > > -	case ARPHRD_LOOPBACK:
> > > -		rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > -				     skb->len);
> > > -		if (rc > 0)
> > > -			rc = 0;
> > > -		break;
> > > -	default:
> > > -		break;
> > > -	}
> > > -	return rc;
> > > -}  
> 
> There's also an EXPORT somewhere in this file that has to go.
> 
> > >  /**
> > >   *	llc_build_and_send_ui_pkt - unitdata request interface for upper layers
> > >   *	@sap: sap to use    
> > 
> > You may break other uses of LLC.
> > 
> > Why not open code as different function.  I used the llc stuff because there
> > were multiple copies of same code (DRY).  
> 
> I didn't quite get what you mean, Stephen, would you mind restating?


Short answer: it was idea that didn't pan out.

Suggestion: get rid of using LLC  in bridge and just rewrite that one place.