On Fri, Feb 24, 2017 at 11:55:37AM -0800, Stephen Hemminger wrote:
> The concept is fine.
Thanks for taking a look.
> Please add some comments to the code about what is happening and why.
> The proposed patch is too sparse and has no comments.
Sure, will do that for the next version.
> > + skb = alloc_skb(hlen + sizeof(struct ipv6hdr) + sizeof(*msg) +
> > + ndisc_opt_addr_space(dev,
> > +NDISC_NEIGHBOUR_ADVERTISEMENT) +
> > + tlen, GFP_ATOMIC);
> > + if (!skb)
> > + return;
>
> Why not netdev_alloc_skb which takes care of padding and setting skb->dev?
This implementation in br_ndisc_send_na() was trying to follow
ndisc_send_na() design for the operations.. If this function remains
(see below), I can clean this up further.
> Rather than doing copy/paste of the code to generate a ND message, it would
> be better to have one function in IPv6 code that handles that. That would keep
> from having to fix code in two places in the future. Is there some way
> to extend ndisc_send_na?
That was the original plan and adding the target_lladdr part would be
straightforward. The part that gets complex is in figuring out how to
use a foreign link layer source address (the MAC address on behalf of
which the local device is replying) in the outgoing NA when using the
IEEE 802.11/Hotspot 2.0 design.
ndisc_send_na() uses the full IPv6 stack for building the frame when
calling ndisc_send_skb(). dst_output() ends up sending this through
ip6_output(), I'd assume, and after building the IPv6 header, the local
MAC address of the outgoing interface gets assigned to the Ethernet
header. I'm not sure how to override that functionality in any clean
way. The dev_hard_header() call in the mostly copy-pasted version in
br_ndisc_send_na() followed by use of the custom
br_ndisc_send_na_finish() to call dev_queue_xmit(skb) was done to allow
the link layer source address to be modified.
The normal path in the net stack seemed to use dev_hard_header() with
saddr = NULL which maps to eth_header() saddr = NULL case to use device
source address. Either those would need to be somehow modified for this
special skb containing the NA with different source address requirement
or something after these calls would need to modify the frame to change
the source address.
Would you happen to know any convenient means for modifying the IPv6
stack behavior for ndisc_send_skb() cases conditionally to allow the
link layer source address to be modified while still being able to use
the existing IPv6 header and the Ethernet header construction function?
--
Jouni MalinenPGP id EFC895FA
