[ovs-dev] [PATCH V3 05/14] netdev-offload-dpdk: Implement HW miss packet recover for vport

[Ivan Malov]

Hi Eli,

On 02/03/2021 14:25, Eli Britstein wrote:
 > +        /* For tunnel recovery (RTE_FLOW_RESTORE_INFO_TUNNEL), it is 
possible
 > +         * to have the packet to still be encapsulated, or not
 > +         * (RTE_FLOW_RESTORE_INFO_ENCAPSULATED).
 > +         * In the case it is on, the packet is still encapsulated, 
and we do
 > +         * the pop in SW.
 > +         * In the case it is off, the packet is already decapsulated 
by HW, and
 > +         * the tunnel info is provided in the tunnel struct. For 
this case we
 > +         * take it to OVS metadata.
 > +         */
 > +        if (rte_restore_info.flags & 
RTE_FLOW_RESTORE_INFO_ENCAPSULATED) {

The highlighted code is located *after* 
netdev_dpdk_rte_flow_get_restore_info() invocation. The comment says 
that the packet may *still* be encapsulated. Is the packet in fact 
allowed to have encapsulation header in it even *before* 
netdev_dpdk_rte_flow_get_restore_info()? I.e. right from the point where 
the packet is read from an ethdev's Rx queue. Is that legitimate?

What concerns me is that in patch [06/14] TCP flags get parsed before 
the miss recovery:
 > +    /* Restore the packet if HW processing was terminated before 
completion. */
 > +    *tcp_flags = parse_tcp_flags(packet);
 > +    p = pmd_send_port_cache_lookup(pmd, port_no);
 > +    if (OVS_LIKELY(p)) {
 > +        int err = netdev_hw_miss_packet_recover(p->port->netdev, 
packet);

So, I wonder, if the packet is indeed allowed to have encapsulation 
header at that point, can parse_tcp_flags() tolerate that? Will the 
flags lookup still be fine?

(I apologise for asking too many questions).

-- 
Ivan M