Ilya Maximets <i.maxim...@ovn.org> 于2022年2月12日周六 05:34写道：
> On 1/28/22 17:14, Aaron Conole wrote:
> > From: Peng He <xnhp0...@gmail.com>
> >
> > 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.")
>
> Nit: please, don't wrap the tags.
> I'm sorry, but I'm getting trouble relating the mentioned commit
> with the code change here. Commit 1df7f7aac8 made only cosmetic
> change to the 'execution' path in order to split out the function.
> The real change was made only for the packet resume, which doesn't
> seem to be the case for the current patch.
>
In the commit 1df7f7aac8, it adds a function to set the correct in_port
in the packet md:
+static void
+ofproto_dpif_set_packet_odp_port(const struct ofproto_dpif *ofproto,
+ ofp_port_t in_port, struct dp_packet
*packet)
+{
+ if (in_port == OFPP_NONE) {
+ in_port = OFPP_LOCAL;
+ }
+ packet->md.in_port.odp_port = ofp_port_to_odp_port(ofproto, in_port);
+}
+
however it forgets to change the in_port field in the flow struct. This is
ok when
the ipf_ctx patch is not added. If the ipf_ctx is added, it will use flow's
in_port as
part of ipf_ctx.
If this patch is not added, the ipf_ctx patch will break some testsuites
which uses
packetout to inject a test packet.
> I managed to trace the in_port update in the 'execution' path down
> to commit 758c456df570 ("dpif: Use explicit packet metadata.").
> It significantly changed the way code is structured, so it's more
> likely a commit to blame.
>
> I don't think that commit message here makes sense. So, I'm not
> sure what exactly this patch is fixing. Am I missing something?
>
> I think, we need a new commit message that describes what actually
> the problem is and how it affects packet processing. And we
> definitely need a unit test for that patch.
>
> Best regards, Ilya Maximets.
>
> > Signed-off-by: Peng He <hepeng.0...@bytedance.com>
> > Acked-by: Mike Pattrick <m...@redhat.com>
> > Signed-off-by: Aaron Conole <acon...@redhat.com>
> > ---
> > ofproto/ofproto-dpif.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 8143dd965f..fed037b8d9 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -5011,6 +5011,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;
> > }
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
--
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
