Jiri Benc <jb...@redhat.com> writes: > On Wed, 23 Sep 2015 12:42:30 -0500, Eric W. Biederman wrote: [snip] >> So perhaps instead of: >> + if (arp->ar_op == htons(ARPOP_REQUEST) && skb_metadata_dst(skb)) >> + reply_dst = (struct dst_entry *) >> + iptunnel_metadata_reply(skb_metadata_dst(skb), >> + GFP_ATOMIC); >> + >> The code would be: >> if (arp->ar_op == htons(AROP_REQUEST) && dev->ndo_reply_dst) >> reply_dst = dev->ndo_reply_dst(skb, GFP_ATOMIC); > > This is something I intended to do originally. I also considered adding > the callback into metadata_dst itself. However, it doesn't make much > sense for the time being.
If you convert this into the equivalent for neighbour discovery where the ingress metadata dst has already been lost it makes a lot of sense. Which is the question you were asking that I have been replying to as I think this all through. [snip] > If there's another user of metadata_dst in the future, this needs to be > changed anyway. We can add such ndo callback (or some other kind of > callback) then, if it turns out to be needed. For now, we don't need it > and the changes to make metadata_dst usable for other things than IP > tunnels are not suitable for net.git. I think you were misunderstanding what I was suggesting. I was suggesting an ndo callback that does not need an metadata dst on the skb but has enough information between the network device and the skb itself to properly construct a dst that can be used for arp and ndisc type replies where you want to exactly reverse the path. *Blink* You were targeting net.git with a feature enhancement???? I will just ignore that. >> For any network that does interesting things with network level >> identifiers below IP this seems like a general problem. Further it >> seems more desirable to only perform an allocation when necessary, >> rather than for every packet, and two for the packets that actually >> need replies. > > The metadata_dst are only allocated when requested. Look at e.g. > vxlan_collect_metadata function. If they're requested, it means they are > needed. And they certainly need to be allocated for ARP replies to such > packets. I don't see what could be further optimized here. Seems to me > that you assume that the sole purpose of why metadata_dst exist is ARP > which is not the case. What I was observing is that in general the only tunneled packets that need an ingress metadata dst for a tunneled medium ethernet like medium are arp and ndisc packets. In other cases if you aren't doing something exceptional like openvswitch the normal routing should be sufficient. Which means a ndo_reply_dst method could remove the need in many cases for an ingress metadata dst to need to be allocated. Regardless a netdevice operation that digs into the packet and figures out what is necessary for a reply seems like the clean way to make this work for both arp and neighbour discovery. Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
