svn commit: r260488 - head/sys/net

[Alexander V. Chernikov]

On 10.01.2014 14:14, Gleb Smirnoff wrote:
>    Alexander,
>
>    some nitpicking:
>
> On Thu, Jan 09, 2014 at 06:13:25PM +0000, Alexander V. Chernikov wrote:
> A> @@ -52,6 +53,7 @@
> A>  #include <sys/proc.h>
> A>  #include <sys/domain.h>
> A>  #include <sys/kernel.h>
> A> +#include <sys/kdb.h>
> A>
> A>  #include <net/if.h>
> A>  #include <net/if_var.h>
> A> @@ -86,6 +88,13 @@
> A>  #define	RT_NUMFIBS	1
> A>  #endif
> A>
> A> +#if defined(INET) || defined(INET6)
> A> +#ifdef SCTP
> A> +extern void sctp_addr_change(struct ifaddr *ifa, int cmd);
> A> +#endif /* SCTP */
> A> +#endif
> A> +
> A> +
>
> Can be simplified to one liner:
>
> #if (defined(INET) || defined(INET6)) && defined(SCTP)
>
> Same stands for same ifdef down below in code.
>
> And extra empty line shouldn't have been added.
This actually needs to be removed from here and converted to be 
ifaddr_event consumer.

>
> A> +
> A> +/*
> A> + * Announce interface address arrival/withdraw
> A> + * Returns 0 on success.
> A> + */
> A> +int
> A> +rt_addrmsg(int cmd, struct ifaddr *ifa, int fibnum)
> A> +{
> A> +
> A> +	KASSERT(cmd == RTM_ADD || cmd == RTM_DELETE,
> A> +		("unexpected cmd %u", cmd));
> A> +	
> A> +	if (fibnum != RT_ALL_FIBS) {
> A> +		KASSERT(fibnum >= 0 && fibnum < rt_numfibs, ("%s: "
> A> +		    "fibnum out of range 0 <= %d < %d", __func__,
> A> +		     fibnum, rt_numfibs));
> A> +	}
> A> +
> A> +	return (rtsock_addrmsg(cmd, ifa, fibnum));
> A> +}
>
> Second KASSERT together with if clause can be simplified to:
>
> KASSERT(fibnum == RT_ALL_FIBS || (fibnum >= 0 && fibnum < rt_numfibs), ...
>
> Same simplification can be done in rt_routemsg() and rt_newaddrmsg_fib().
Yes, thanks.
>
> A> +
> A> +/*
> A> + * Announce route addition/removal
> A> + * Users of this function MUST validate input data BEFORE calling.
> A> + * However we have to be able to handle invalid data:
> A> + * if some userland app sends us "invalid" route message (invalid mask,
> A> + * no dst, wrokg address families, etc...) we need to pass it back
> 		  ^
> typo
Fixed.
>
> A> + * to app (and any other rtsock consumers) with rtm_errno field set to
> A> + * non-zero value.
> A> + * Returns 0 on success.
> A> + */
>
> A> +int
> A> +rtsock_routemsg(int cmd, struct ifnet *ifp, int error, struct rtentry *rt,
> A> +    int fibnum)
> A>  {
> A> +	struct rt_addrinfo info;
> A> +	struct sockaddr *sa;
> A> +	struct mbuf *m;
> A> +	struct rt_msghdr *rtm;
> A>
> A> -	rt_newaddrmsg_fib(cmd, ifa, error, rt, RT_ALL_FIBS);
> A> +	if (route_cb.any_count == 0)
> A> +		return (0);
> A> +
> A> +	bzero((caddr_t)&info, sizeof(info));
> A> +	info.rti_info[RTAX_NETMASK] = rt_mask(rt);
> A> +	info.rti_info[RTAX_DST] = sa = rt_key(rt);
> A> +	info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
> A> +	if ((m = rt_msg1(cmd, &info)) == NULL)
> A> +		return (ENOBUFS);
> A> +	rtm = mtod(m, struct rt_msghdr *);
> A> +	rtm->rtm_index = ifp->if_index;
> A> +	rtm->rtm_flags |= rt->rt_flags;
> A> +	rtm->rtm_errno = error;
> A> +	rtm->rtm_addrs = info.rti_addrs;
> A> +
> A> +	if (fibnum != RT_ALL_FIBS) {
> A> +		M_SETFIB(m, fibnum);
> A> +		m->m_flags |= RTS_FILTER_FIB;
> A> +	}
> A> +
> A> +	rt_dispatch(m, sa ? sa->sa_family : AF_UNSPEC);
> A> +
> A> +	return (0);
> A>  }
> A>
> A> +
>
> Why extra line here?
Fixed.
>
> A>  /*
> A>   * This is the analogue to the rt_newaddrmsg which performs the same
> A>   * function but for multicast group memberhips.  This is easier since
>