Yes, I agree. It should be the way to go. Thanks, Yifeng
On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose <[email protected]> wrote: > On 8/9/2018 11:03 AM, Yifeng Sun wrote: > > There is a difference regarding how skb_tunnel_info is stored in skb > between ovs and upstream kernel. In ovs's compatible gre module, > skb_tunnel_info is stored in skb->cb while in upstream kernel, it is > referenced by skb->_skb_refdst. > > The upstream netdev code should be okay. > > To fix this issue, my guess is that either we comply to the kernel's way > by using skb->_skb_refdst to store skb_tunnel_info, or we don't use > IPCB at all. > > > Ah, I see... > > We must comply with the kernel method for any given kernel. We can't be > sure that we'll only handle > or own packets. > > Can't this be fixed by a compatibility layer #define in acinclude.m4 so > that kernels that store it in > skb->cb vs. kernels that store it in skb->__skb_refdst will both work? > > Thanks, > > - Greg > > > > Thanks, > Yifeng > > > > On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose <[email protected]> > wrote: > >> >> On 8/8/2018 6:50 PM, William Tu wrote: >> >>> thanks for the fix. >>> >>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun <[email protected]> >>> wrote: >>> >>> In compatable gre module, skb->cb is used as ovs_gso_cb. >>>> This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst. >>>> >>>> can you explain more about ovs_gso_cb? >>> >>> Signed-off-by: Yifeng Sun <[email protected]> >>>> --- >>>> datapath/linux/compat/ip6_gre.c | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/datapath/linux/compat/ip6_gre.c >>>> b/datapath/linux/compat/ip6_ >>>> gre.c >>>> index 54a76ab..16c1f72 100644 >>>> --- a/datapath/linux/compat/ip6_gre.c >>>> +++ b/datapath/linux/compat/ip6_gre.c >>>> @@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct >>>> sk_buff *skb, >>>> goto tx_err; >>>> >>>> t->parms.o_flags &= ~TUNNEL_KEY; >>>> - IPCB(skb)->flags = 0; >>>> >>>> The upstream linux kernel has the above code. >>> Do we need to fix the upstream kernel then backport? >>> >> >> That would be the normal work flow. >> >> Yifeng, >> >> Can you please post a patch with this fix to netdev? Taking William's >> comments into account as well. >> >> Good catch and thanks for the fix! >> >> - Greg >> >> >>> Thanks, >>> William >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
