[Bridge] [PATCH] decouple llc/bridge
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
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
+#include
#include
#include
#include
@@ -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
#include
-/**
- * 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: [Bridge] [PATCH] decouple llc/bridge
On Thu, 7 Apr 2022 19:48:59 -0700
Jakub Kicinski 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
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
On Thu, 7 Apr 2022 11:12:17 -0400
Dave Jones 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
>
> 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
> +#include
> #include
> #include
> #include
> @@ -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
> #include
>
> -/**
> - * 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).
