On Fri, Jan 12, 2018 at 05:43:13PM +0000, Bhanuprakash Bodireddy wrote: > During tunnel decapsulation the below steps are performed: > [1] Tunnel information is populated in packet metadata i.e > packet->md->tunnel. > [2] Outer header gets popped. > [3] Packet is recirculated. > > For [1] to work, the dp_packet L3 and L4 header offsets should be valid. > The offsets in the dp_packet are set as part of miniflow extraction. > > If offsets are accidentally reset (or) the pop header operation is performed > prior to miniflow extraction, step [1] fails silently and creates > issues that are harder to debug. Add the assertion to check if the > offsets are valid. > > Signed-off-by: Bhanuprakash Bodireddy <[email protected]> > --- > lib/netdev-native-tnl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c > index 9ce8567..fb5eab0 100644 > --- a/lib/netdev-native-tnl.c > +++ b/lib/netdev-native-tnl.c > @@ -508,6 +508,9 @@ netdev_vxlan_pop_header(struct dp_packet *packet) > ovs_be32 vx_flags; > enum packet_type next_pt = PT_ETH; > > + ovs_assert(packet->l3_ofs > 0); > + ovs_assert(packet->l4_ofs > 0); > + > pkt_metadata_init_tnl(md); > if (VXLAN_HLEN > dp_packet_l4_size(packet)) { > goto err;
Thanks for working to make OVS more reliable. How much risk do you think there is of these assertions triggering? Are you debugging an issue where they would trigger, and has that been fixed? I'm trying to figure out whether it makes more sense to put assertions here or whether something closer to a log message plus a jump to "err" would be better. It's not great for OVS to assert-fail, but on the other hand if it indicates a genuine bug then sometimes it's the best thing to do. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
