On Fri, Sep 18, 2015 at 6:36 PM, John W. Linville <linvi...@tuxdriver.com> wrote: > On Fri, Sep 18, 2015 at 02:49:30PM -0700, Jesse Gross wrote: >> On Fri, Sep 18, 2015 at 1:40 PM, John W. Linville >> <linvi...@tuxdriver.com> wrote: >> > On Fri, Sep 18, 2015 at 01:30:36PM -0700, Jesse Gross wrote: >> >> On Thu, Sep 17, 2015 at 1:34 PM, John W. Linville >> >> <linvi...@tuxdriver.com> wrote: >> >> > This seems to have been a "thinko". IP_ECN_decapsulate needs info >> >> > from both internal and external headers. >> >> > >> >> > Signed-off-by: John W. Linville <linvi...@tuxdriver.com> >> >> >> >> This looks good to me although I realized that the transmit path is >> >> also conditional based on !collect_md metadata. I suppose that means >> >> that this difference is intentional and I guess I could see arguments >> >> either way but I still think it is better to be consistent across >> >> different code paths in this respect. >> > >> > I'm happy with the v2 patch if you are. :-) >> >> I'm happy with this part of the code path after this patch. However, >> my concern was that this patch, while correct, will make receive >> inconsistent with transmit. Presumably we should update the transmit >> side as well. > > Oh, sorry -- I missed your point. You mean something like this? > > @@ -812,19 +815,16 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, > struct net_device *dev) > if (unlikely(err)) > goto err; > > - tos = key->tos; > + tos = ip_tunnel_ecn_encap(key->tos, iip, skb); > ttl = key->ttl; > df = key->tun_flags & TUNNEL_DONT_FRAGMENT ? htons(IP_DF) : 0; > > Am I understanding now?
Yes, essentially though I think that 'iip' is only defined in the latter half of the if statement and it might be nice to have the ECN encapsulation called only once before udp_tunnel_xmit_skb() rather than independently in each code path. -- 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