Re: [lwip-devel] Non-broadcast interfaces (patch)

[Leon Woestenberg]

Hello Tony,

> Initially I set up a SLIP interface, which I connected to a SLIP port on a
> Linux box. The standard subnet mask for a SLIP link is 255.255.255.255, so
I
> specified this in lwip when setting up the interface. With this subnet
mask,
>
Is this standard SLIP subnet mask specified somewhere? I would expect a
0.0.0.0 network mask on a point-to-point link.

> the test macro ip_addr_isbroadcast() always evaluates to TRUE, which was
> causing all incoming packets to be discarded.
>
TCP packets only, I hope? Broadcast IP traffic should pass at the IP layer.
(ah yes, your patch explains :)

> I think this test should only
> be done if the interface is broadcast-capable (e.g. ethernet), e.g.
>
> if (((inp->flags & NETIF_FLAG_BROADCAST) &&
>       ip_addr_isbroadcast(&(iphdr->dest), &(inp->netmask))) ||
>       ip_addr_ismulticast(&(iphdr->dest))) {
>
> Also, slipif_init() should include:
>
> netif->flags = NETIF_FLAG_POINTTOPOINT;
>
> I have attached a patch file with changes for this to icmp.c, tcp_in.c,
> udp.c and slipif.c - please could someone evaluate it and add to CVS?
Thanks
>
Your patch would work. I have looked into the NetBSD code, and it does the
same, *plus* more:

Basically, it says that an address that exactly matches the network
interface address,
is not a broadcast address.

However, the BSD broadcast check function has an extra argument, which is
the
interface itself. (BTW, NetBSD's in_broadcast() attached below).

So, for lwIP, we might change the broadcast check function to match BSD's,
OR
do the network interface check in the caller's context (every caller will
then have
two extra conditions built-in).

I would strongly suggest the former approach, as it hides the two extra
checks
in the function, making it less error-prone and easier maintainable.

BTW, patches are most welcome in the Savannah patches page. It allows us
to track patches more easily.

Also, are you interested in contributing your port to the "contrib/ports"
area of our
CVS repository, or is this closed contract work?

Regards,

Leon.


/*      $NetBSD: in.c,v 1.92 2003/10/23 20:55:08 mycroft Exp $  */

/*
 * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the project nor the names of its contributors
 *    may be used to endorse or promote products derived from this software
 *    without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE PROJECT AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE PROJECT OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

/*<cut out, see
http://cvsup.pt.freebsd.org/cgi-bin/cvsweb/cvsweb.cgi/src/sys/netinet/in.c?cvsroot=NetBSD&rev=1.93>

/*
 * Return 1 if the address might be a local broadcast address.
 */
int
in_broadcast(in, ifp)
        struct in_addr in;
        struct ifnet *ifp;
{
        struct ifaddr *ifa;

        if (in.s_addr == INADDR_BROADCAST ||
            in_nullhost(in))
                return 1;
        if ((ifp->if_flags & IFF_BROADCAST) == 0)
                return 0;
        /*
         * Look through the list of addresses for a match
         * with a broadcast address.
         */
#define ia (ifatoia(ifa))
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list)
                if (ifa->ifa_addr->sa_family == AF_INET &&
                    !in_hosteq(in, ia->ia_addr.sin_addr) &&
                    (in_hosteq(in, ia->ia_broadaddr.sin_addr) ||
                     in_hosteq(in, ia->ia_netbroadcast) ||
                     (hostzeroisbroadcast &&
                      /*
                       * Check for old-style (host 0) broadcast.
                       */
                      (in.s_addr == ia->ia_subnet ||
                       in.s_addr == ia->ia_net))))
                        return 1;
        return (0);
#undef ia
}