On 8/9/2018 11:36 AM, Yifeng Sun wrote:
Yes, I agree. It should be the way to go.
Thanks,
Yifeng
If I'm reading the code correctly the OVS packet cmd will set the
skb->cb using the OVS_CB macro and then
pass that packet down to the ip6erspan_tunnel_xmit function. The
ip6erspan_tunnel_xmit function will
then use the IPCB macro to access the skb->cb area to set the flag to
zero. That works fine for packets
generated by the system that have used the IPCB macro accessor to set
the skb->cb data. However, it
is catastrophic when an skb was prepared with the OVS_CB macro because,
as you might imagine, it
is not pointing to an inet_skb_parm structure but an ovs_skb_cb
structure instead.
This code was *never* correct. Somehow or other it didn't cause a
problem until 4.15. We'll have to
figure out some way to check if the packet is ours or is system
generated to know if we should be using
the IPCB accessor.
In instances of packets generated by the system or any other entity
transmitting ipv6 packets over the
tunnel we'll need to keep the code that clears the flags.
Thanks,
- Greg
On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose <[email protected]
<mailto:[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] <mailto:[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] <mailto:[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]
<mailto:[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] <mailto:[email protected]>
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
