On Thu, Jun 23, 2011 at 11:33:50AM +1000, David Gwynne wrote:
>
> On 22/06/2011, at 3:52 AM, Stefan Rinkes wrote:
>
> > 2. If you are using IP balanced carp and set one of
> > the interface to down, the mbufs still reach pf.
> > Cause carp_ourether() returns NULL if the interface
> > is down and the mbufs gets copied by carp_input(),
> > cause the M_MCAST flag is set. The copied mbuf is
> > dropped in ether_input() since the carp interface is down
> > and the original mbuf reaches the pf. IMHO carp should always
> > take care of mbufs with his MAC address, else the machine has
> > to do some unnecessary work.
> >
> > retrieving revision 1.184
> > diff -u -p -r1.184 ip_carp.c
> > --- sys/netinet/ip_carp.c 4 May 2011 16:05:49 -0000 1.184
> > +++ sys/netinet/ip_carp.c 21 Jun 2011 17:34:42 -0000
> > @@ -1514,9 +1514,7 @@ carp_ourether(void *v, struct ether_head
> >
> > TAILQ_FOREACH(vh, &cif->vhif_vrs, sc_list) {
> > struct carp_vhost_entry *vhe;
> > - if ((vh->sc_if.if_flags & (IFF_UP|IFF_RUNNING)) !=
> > - (IFF_UP|IFF_RUNNING))
> > - continue;
> > +
> > if (vh->sc_balancing == CARP_BAL_ARP) {
> > LIST_FOREACH(vhe, &vh->carp_vhosts, vhost_entries)
> > if (vhe->state == MASTER &&
> >
>
> this looks reasonable to me. mcbride, mpf, could you chip in on this?
Hi,
I don't like this.
While this might be ok for load balanced setups, it complicates
the code for standard carp cases. Because it will now rely on
vhe->state being INIT when you ifconfig down the interface.
IMO, the problem is more fundamental.
On the input path, our stack should follow RFC 1122 more closely
and drop unicast IP packets that have been received via a link
layer broadcast address.
Section 3.3.6 states:
A host SHOULD silently discard a datagram that is received via
a link-layer broadcast (see Section 2.4) but does not specify
an IP multicast or broadcast destination address.
If we have an IP balanced carp interface, we explicitly allow that
of course ;-)
I haven't looked into the v6 case, but I guess it would need sth
similar.
Index: ip_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.192
diff -p -u -p -u -r1.192 ip_input.c
--- ip_input.c 15 Jun 2011 09:11:01 -0000 1.192
+++ ip_input.c 4 Jul 2011 02:11:29 -0000
@@ -309,6 +309,16 @@ ipv4_input(m)
goto bad;
}
}
+ /*
+ * Discard link layer broadcasts that do not specify
+ * an IP multicast or broadcast address - RFC1122 3.3.6
+ */
+ if (ISSET(m->m_flags, (M_BCAST|M_MCAST)) &&
+ !(IN_MULTICAST(ip->ip_dst.s_addr) ||
+ ip->ip_dst.s_addr == INADDR_BROADCAST)) {
+ ipstat.ips_badaddr++;
+ goto bad;
+ }
if ((m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_OK) == 0) {
if (m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_BAD) {
