| Message ID | 56868207.rHBDZL3pbk@wuerfel |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jan 13, 2015 at 6:10 AM, Arnd Bergmann <arnd@arndb.de> wrote: > When IPV4 support is disabled, we cannot call arp_send from > the bridge code, which would result in a kernel link error: > > net/built-in.o: In function `br_handle_frame_finish': > :(.text+0x59914): undefined reference to `arp_send' > :(.text+0x59a50): undefined reference to `arp_tbl' > > This makes the newly added proxy ARP support in the bridge > code depend on the CONFIG_INET symbol and lets the compiler > optimize the code out to avoid the link error. > Not sure how much sense to make CONFIG_BRIDGE depend on CONFIG_INET, at least CONFIG_BONDING does. -- 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
From: Cong Wang <cwang@twopensource.com> Date: Tue, 13 Jan 2015 11:25:45 -0800 > On Tue, Jan 13, 2015 at 6:10 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> When IPV4 support is disabled, we cannot call arp_send from >> the bridge code, which would result in a kernel link error: >> >> net/built-in.o: In function `br_handle_frame_finish': >> :(.text+0x59914): undefined reference to `arp_send' >> :(.text+0x59a50): undefined reference to `arp_tbl' >> >> This makes the newly added proxy ARP support in the bridge >> code depend on the CONFIG_INET symbol and lets the compiler >> optimize the code out to avoid the link error. >> > > Not sure how much sense to make CONFIG_BRIDGE depend > on CONFIG_INET, at least CONFIG_BONDING does. It depends upon whether we want to provide and consider as a valid configuration bridging without INET. Probably we do. -- 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
On 1/13/15 1:57 PM, David Miller wrote: > From: Cong Wang <cwang@twopensource.com> > Date: Tue, 13 Jan 2015 11:25:45 -0800 > >> On Tue, Jan 13, 2015 at 6:10 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> When IPV4 support is disabled, we cannot call arp_send from >>> the bridge code, which would result in a kernel link error: >>> >>> net/built-in.o: In function `br_handle_frame_finish': >>> :(.text+0x59914): undefined reference to `arp_send' >>> :(.text+0x59a50): undefined reference to `arp_tbl' >>> >>> This makes the newly added proxy ARP support in the bridge >>> code depend on the CONFIG_INET symbol and lets the compiler >>> optimize the code out to avoid the link error. >>> >> >> Not sure how much sense to make CONFIG_BRIDGE depend >> on CONFIG_INET, at least CONFIG_BONDING does. > > It depends upon whether we want to provide and consider > as a valid configuration bridging without INET. Probably > we do. Rather than connect CONFIG_BRIDGE to CONFIG_INET, why not make br_do_proxy_arp (and setting BR_PROXYARP flag) a no-op if CONFIG_INET is not set? #ifdef CONFIG_INET #else static inline void br_do_proxy_arp(...args...) { } #endif That covers both arp_tbl and arp_send. 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
On Tuesday 13 January 2015 14:14:20 David Ahern wrote: > > Rather than connect CONFIG_BRIDGE to CONFIG_INET, why not make > br_do_proxy_arp (and setting BR_PROXYARP flag) a no-op if CONFIG_INET is > not set? > > #ifdef CONFIG_INET > #else > static inline void br_do_proxy_arp(...args...) > { > } > #endif > > That covers both arp_tbl and arp_send. The effect is very similar to my patch (probably same object code), the only difference should be that it would add an ugly #ifdef instead of the preferred IS_ENABLED() check, so you don't get any compile-time coverage of the function. It's not really important because everybody has CONFIG_INET enabled in practice and it does get more than enough compile-time coverage. Arnd -- 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
On 1/13/15 2:33 PM, Arnd Bergmann wrote: > The effect is very similar to my patch (probably same object code), the > only difference should be that it would add an ugly #ifdef instead of > the preferred IS_ENABLED() check, so you don't get any compile-time > coverage of the function. Indeed. As long as br_do_proxy_arp does not get exported that works the same. 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
From: Arnd Bergmann <arnd@arndb.de> Date: Tue, 13 Jan 2015 15:10:27 +0100 > When IPV4 support is disabled, we cannot call arp_send from > the bridge code, which would result in a kernel link error: > > net/built-in.o: In function `br_handle_frame_finish': > :(.text+0x59914): undefined reference to `arp_send' > :(.text+0x59a50): undefined reference to `arp_tbl' > > This makes the newly added proxy ARP support in the bridge > code depend on the CONFIG_INET symbol and lets the compiler > optimize the code out to avoid the link error. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 958501163ddd ("bridge: Add support for IEEE 802.11 Proxy ARP") Applied, thanks Arnd. -- 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
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 1f1de715197c..e2aa7be3a847 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -154,7 +154,8 @@ int br_handle_frame_finish(struct sk_buff *skb) dst = NULL; if (is_broadcast_ether_addr(dest)) { - if (p->flags & BR_PROXYARP && + if (IS_ENABLED(CONFIG_INET) && + p->flags & BR_PROXYARP && skb->protocol == htons(ETH_P_ARP)) br_do_proxy_arp(skb, br, vid);
When IPV4 support is disabled, we cannot call arp_send from the bridge code, which would result in a kernel link error: net/built-in.o: In function `br_handle_frame_finish': :(.text+0x59914): undefined reference to `arp_send' :(.text+0x59a50): undefined reference to `arp_tbl' This makes the newly added proxy ARP support in the bridge code depend on the CONFIG_INET symbol and lets the compiler optimize the code out to avoid the link error. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 958501163ddd ("bridge: Add support for IEEE 802.11 Proxy ARP") Cc: Kyeyoon Park <kyeyoonp@codeaurora.org> -- 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