Ilya Maximets <i.maxim...@ovn.org> 于2022年2月15日周二 01:42写道:

> On 2/14/22 18:32, Aaron Conole wrote:
> > Peng He <xnhp0...@gmail.com> writes:
> >
> >> In the commit 1df7f7aac8c976690167261fec9a50d832ef9e7e, the packet
> >> metadata's in_port has been changed from ofp port into odp port,
> >> however, the odp->flow->in_port is still ofp_port. This patch changes
> >> the odp->flow->in_port into odp_port.
> >>
> >> Fixes: 1df7f7aac8 ("ofproto-dpif: Restore packet metadata when a
> >> continuation is resumed.")
> >
> > This tag is still wrapped, and I don't think the fixes tag is for the
> > correct commit.
>
weird. the link:
http://patchwork.ozlabs.org/project/openvswitch/patch/20220213023642.4248-1-hepeng.0...@bytedance.com/
shows the tag is not wrapped. If by tag, you mean the line "Fixes: [commit
id] ("title")".


> >
> >> Signed-off-by: Peng He <hepeng.0...@bytedance.com>
> >> ---
> >>  ofproto/ofproto-dpif.c | 1 +
> >
> > Also, seems there is no unit test as requested.  Can you please address
> > these?
>
> This patch is needed for ipf_ctx to pass through the testsuite.

BTW, If we will not touch the flow struct here, is it ok to touch it at
dpif_execute and finally at dpif_netdev_execute? As it is now in the
datapath ctx and it is ok to assume flow->in_port is the odp port, and
this fix is mainly for the userspace datapath for ipf_ctx.

the reason I prefer using flow->in_port is mainly that it is more
cache efficient to touch a flow struct than packet->md (since you have to
touch
packet metadata, it's changing and more likely not in cahce compared to
flow).
But I am ok if changed to using packet->md.




> I'm not sure the unit test is possible here, as datapath currently
> doesn't use the in_port from the flow structure while executing
> actions.  And taking into account my previous reply here:
>   https://mail.openvswitch.org/pipermail/ovs-dev/2022-February/391570.html
> I'd say that having OpenFlow port number in that structure is fairly
> correct and we need to fix the ipf_ctx patch to use the port number
> from the packet metadata instead.
>
> I'll also post a couple of questions for the ipf_ctx patch.




> >
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> index cba49a99e..72de67c63 100644
> >> --- a/ofproto/ofproto-dpif.c
> >> +++ b/ofproto/ofproto-dpif.c
> >> @@ -4973,6 +4973,7 @@ packet_execute_prepare(struct ofproto *ofproto_,
> >>      ofproto_dpif_set_packet_odp_port(ofproto,
> opo->flow->in_port.ofp_port,
> >>                                       opo->packet);
> >>
> >> +    opo->flow->in_port = opo->packet->md.in_port;
> >>      ofproto_dpif_packet_out_delete(aux);
> >>      opo->aux = execute;
> >>  }
> >
>
>

-- 
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to