Re: Bridged ethernet with ipnat redirect to local port - getting ICMP redirects instead

[Darren Reed <darrenr%netbsd.org@localhost>]

On 7/07/2014 4:49 AM, Thomas Bieg wrote:
> Darren Reed wrote:
>> This might be a better patch..
> 
> Indeed, thanks, but unfortunately I don't see any change with it; am still
> getting the ICMP redirects (real target IP replaced by 1.2.3.4):
> 
> 20:33:06.954296 IP (tos 0x0, ttl 64, id 5501, offset 0, flags [none], proto 
> ICMP (1), length 56)
>     1.2.3.4 > 192.168.111.106: ICMP redirect 1.2.3.4 to host 192.168.111.1, 
> length 36

The attached patch should sort things out for you.

However in testing I discovered that the interaction between IP and bridging
to be really broken leading me to suspect that there is something more
fundamentally wrong.

For example, without this patch, with a VM running inside ESXi, if I do this:

ifconfig bridge0 create
brconfig bridge0 add wm0

(where wm0 has an IP address assigned to it)

as the first step in bridging wm0 with wm1 then NetBSD starts issuing ICMP
redirects for traffic that it never should. Again this patch shuts down all
of those ICMP redirects but really the problem here is that the IP input code
path is being called for packets that it should not be.

Now it might simply be that the bridge code has been written to only work
when there are no IP addresses assigned to any of the network interfaces
assigned to the bridge. If that's the case then your use (and my use above)
is outside of the scope for supported bridging with NetBSD.

Unfortunately there is nothing in bridge(4) to say that bridging is or isn't
supported when a network interface has an IP address.

kern/48971

Cheers,
Darren

--- /sys/net/if_bridgevar.h.orig        2012-08-23 22:06:32.000000000 +1000
+++ /sys/net/if_bridgevar.h     2014-07-07 15:42:44.000000000 +1000
@@ -306,6 +306,8 @@
 
 extern const uint8_t bstp_etheraddr[];
 
+bool   bridge_belongsto(void *, struct ifnet *);
+
 void   bridge_ifdetach(struct ifnet *);
 
 int    bridge_output(struct ifnet *, struct mbuf *, const struct sockaddr *,
--- /sys/net/if_bridge.c.orig   2014-07-08 02:05:51.000000000 +1000
+++ /sys/net/if_bridge.c        2014-07-08 02:04:43.000000000 +1000
@@ -1993,6 +1993,26 @@
        splx(s);
 }
 
+/*
+ * bridge_belongsto:
+ *
+ *     Does the ifnet belong to the bridge in question?
+ */
+bool
+bridge_belongsto(void *arg, struct ifnet *ifp)
+{
+       bool result;
+       int s;
+
+       if (arg == NULL || ifp == NULL)
+               return false;
+
+       s = splnet();
+       result = (bridge_lookup_member(arg, ifp->if_xname) != NULL);
+       splx(s);
+       return result;
+}
+
 #if defined(BRIDGE_IPF)
 extern pfil_head_t *inet_pfil_hook;                 /* XXX */
 extern pfil_head_t *inet6_pfil_hook;                /* XXX */
--- /sys/netinet/ip_input.c.orig        2014-07-08 02:07:15.000000000 +1000
+++ /sys/netinet/ip_input.c     2014-07-08 02:07:39.000000000 +1000
@@ -117,6 +117,8 @@
 
 #include <net/if.h>
 #include <net/if_dl.h>
+#include <net/if_ether.h>
+#include <net/if_bridgevar.h>
 #include <net/route.h>
 #include <net/pfil.h>
 
@@ -1223,6 +1225,7 @@
         * or a route modified by a redirect.
         */
        if (rt->rt_ifp == m->m_pkthdr.rcvif &&
+           !bridge_belongsto(rt->rt_ifp->if_bridge, m->m_pkthdr.rcvif) &&
            (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 &&
            !in_nullhost(satocsin(rt_getkey(rt))->sin_addr) &&
            ipsendredirects && !srcrt) {