On 3/15/2021 11:54 PM, Ivan Malov wrote:
External email: Use caution opening links or attachments
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?
The get_restore_info does not change the packet, but just provides the info.
Whether a packet with tunnel info (RTE_FLOW_RESTORE_INFO_TUNNEL) to
still be encapsulated or not (RTE_FLOW_RESTORE_INFO_ENCAPSULATED)
depends on PMD.
Thus, yes, it is 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?
Thanks. I'll fix in v4.
(I apologise for asking too many questions).
NP. Thanks for reviewing.
--
Ivan M
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
