Re: [PATCHv3 net] bonding: fix ICMPv6 header handling when receiving IPv6 messages

[Jay Vosburgh <jay.vosburgh@canonical.com>]

Hangbin Liu <liuhangbin@gmail.com> wrote:

>On Wed, Nov 16, 2022 at 08:29:58PM -0800, Jay Vosburgh wrote:
>> > #if IS_ENABLED(CONFIG_IPV6)
>> >-	} else if (is_ipv6) {
>> >+	} else if (is_ipv6 && skb_header_pointer(skb, 0, sizeof(ip6_hdr), &ip6_hdr)) {
>> > 		return bond_na_rcv(skb, bond, slave);
>> > #endif
>> > 	} else {
>> >
>> >What do you think?
>> 
>> 	I don't see how this solves the icmp6_hdr() / ipv6_hdr() problem
>> in bond_na_rcv(); skb_header_pointer() doesn't do a pull, it just copies
>> into the supplied struct (if necessary).
>
>Hmm... Maybe I didn't get what you and Eric means. If we can copy the
>supplied buffer success, doesn't this make sure IPv6 header is in skb?

	The header is in the skb, but it may not be in the linear part
of the skb, i.e., the header is wholly or partially in a skb frag, not
in the area covered by skb->data ... skb->tail.  The various *_hdr()
macros only look in the linear area, not the frags, and don't check to
see if the linear area contains the entire header.

	skb_header_pointer() is smart enough to check, and if the
requested data is entirely within the linear area, it returns a pointer
to there; if not, it copies from the frags into the supplied struct and
returns a pointer to that.  What it doesn't do is a pull (move data from
a frag into the linear area), so merely calling skb_header_pointer()
doesn't affect the layout of what's in the skb (which is the point,
bonding uses it here to avoid changing the skb).

	There may be better explanations out there, but

http://vger.kernel.org/~davem/skb_data.html

	covers the basics.  Look for the references to "paged data."

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com