Re: [PATCH 1/1] neighbour : fix ndm_type type error issue

From: Jun Zhao
Date: Sat Jul 26 2014 - 11:06:31 EST


Hi, Hannes

On Sat, 2014-07-26 at 15:42 +0200, Hannes Frederic Sowa wrote:
> Hi,
>
> On Sat, Jul 26, 2014, at 02:29, Jun Zhao wrote:
> > On Sat, 2014-07-26 at 01:24 +0200, Hannes Frederic Sowa wrote:
> > > On Fri, Jul 25, 2014, at 18:38, Jun Zhao wrote:
> > > > ndm_type means L3 address type, in neighbour proxy and vxlan, it's
> > > > RTN_UNICAST.
> > > > NDA_DST is for netlink TLV type, hence it's not right value in this
> > > > context.
> > >
> > > The value of NDA_DST == RTN_UNICAST, otherwise we couldn't do this
> > > change as it would alter e.g. arpd behavior.
> > >
> > > Acked-by: Hannes Frederic Sowa <hannes@xxxxxxxxxxxxxxxxxxx>
> > >
> > > Thanks,
> > > Hannes
> >
> > But I think NDA_DST/RTN_UNICAST have different means in this context,
> > even though the value of NDA_DST == RTN_UNICAST.
> >
> > For arp proxy/NDP proxy context, ndm_type means the peer L3 address,
> > so RTN_UNICAST is the right value. For vxlan have similar semantic for
> > remote ip.
> >
> > BTW: In the source code, implicit think NDA_DST == RTN_UNICAST maybe
> > not a good idea when we don't have a comment or the other explain.
>
> I am totally with you and think your change is good, that's why I also
> gave my ack to your patch.
>
> My comment above was about my concerns regarding making a user space
> visible change, which in the end could alter the behavior of already
> existing software.
>
> Developers maybe have debugged code and seen some different value being
> propagated from the kernel and this software could now break if we would
> change the value after all those years.
>
> Bye,
> Hannes

Luckyï NDA_DST == RTN_UNICAST is true really in source code ïï

/* rtm_type */

enum {
RTN_UNSPEC,
RTN_UNICAST,
...
}

enum {
NDA_UNSPEC,
NDA_DST,
...
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/