Ilya Maximets <[email protected]> writes:
> On 5/15/23 17:22, Paolo Valerio wrote:
>> If a packet originating from the controller recirculates after going
>> through a patch port, it gets dropped with the following message:
>>
>> ofproto_dpif_upcall(handler8)|INFO|received packet on unassociated
>> datapath port 4294967295
>>
>> This happens because there's no xport_uuid in the recirculation node
>> and at the same type in_port refers to the patch port.
>>
>> The patch, in the case of zeroed uuid, retrieves the xport starting
>> from the ofproto_uuid stored in the recirc node.
>>
>> Signed-off-by: Paolo Valerio <[email protected]>
>> ---
>> ofproto/ofproto-dpif-xlate.c | 11 +++++++++--
>> tests/ofproto-dpif.at | 34 ++++++++++++++++++++++++++++++++++
>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index c01177718..3509cc73c 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -1533,8 +1533,15 @@ xlate_lookup_ofproto_(const struct dpif_backer
>> *backer,
>>
>> ofp_port_t in_port = recirc_id_node->state.metadata.in_port;
>> if (in_port != OFPP_NONE && in_port != OFPP_CONTROLLER) {
>> - struct uuid xport_uuid = recirc_id_node->state.xport_uuid;
>> - xport = xport_lookup_by_uuid(xcfg, &xport_uuid);
>> + if (uuid_is_zero(&recirc_id_node->state.xport_uuid)) {
>> + const struct xbridge *bridge =
>> + xbridge_lookup_by_uuid(xcfg,
>> &recirc_id_node->state.ofproto_uuid);
>> + xport = bridge ? get_ofp_port(bridge, in_port) : NULL;
>
> IIUC, xport_uuid is designed to not be uuid of the patch port.
> But the in_port here is a patch port, right? So, we will find
> a different xport, right?
>
> Shouldn't we just fall into the else condition that handles
> NONE and CONTROLLER and not look for xport?
>
I guess it's ok to fall in the else in this case.
The only problem is that we'd return the ofproto even if the in_port is
invalid.
This would make in turn fail "conntrack - fragment reassembly with L3 L4
protocol information". This test was fixed in the past after it already
broke once 323ae1e808e6 ("ofproto-dpif-xlate: Fix recirculation when
in_port is OFPP_CONTROLLER.") fixed the use case involving packet-out
and recirculation.
One possibility is to just retrieve the xport for that case in order to
verify the in_port belongs to the bridge, without returning it (so
honoring the xport_uuid logic). Maybe this could be done in the else
branch so to make clear we're handling the special case related to
OFPP_{NONE,CONTROLLER}.
WDYT?
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
