Hi, Mike,
The bug in the last email is not correct.
in packet_execute_prepare functions,
it has changed dp_packet md's port into the odp port,
but in the execute->flow, the value is still the ofp port.
static void
packet_execute_prepare(struct ofproto *ofproto_,
struct ofproto_packet_out *opo)
OVS_REQUIRES(ofproto_mutex)
{
....
pkt_metadata_from_flow(&opo->packet->md, opo->flow);
execute->packet = opo->packet;
execute->flow = opo->flow;
...
/* Fix up in_port. */
ofproto_dpif_set_packet_odp_port(ofproto, opo->flow->in_port.ofp_port,
opo->packet);
....
}
but ipf_ctx is formed using the flow's value instead of packet's value,
this causes the test suite to fail.
the reason of ipf_ctx using the flow's value is at the second time of
calling conntrack_execute, there are no packets in the batch, the batch is
empty and only used as a container for the ipf to inject packets.
I guess maybe we should also change the execute->flow's in_port into the
odp value?
any ideas?
Mike Pattrick <[email protected]> 于2022年1月8日周六 06:03写道：
> On Thu, 2021-12-30 at 08:03 +0000, Peng He wrote:
> > considering a multi-thread PMD setting, when the frags are
> > reassembled
> > in one PMD, another thread might call *ipf_execute_reass_pkts* and
> > 'steal'
> > the reassembled packets into its ipf ctx, then this reassembled
> > packet will enter into another ipf context and causes errors.
> >
> > This happends when there are multiple CT zones, and frags are
> > reassembled in ct(zone=X) might be 'stealed' into the ct(zone=Y).
> >
> > Signed-off-by: Peng He <[email protected]>
> > ---
> > lib/conntrack.c | 2 +-
> > lib/ipf.c | 10 ++++++++--
> > lib/ipf.h | 1 +
> > 3 files changed, 10 insertions(+), 3 deletions(-)
> >
>
> Hello Peng,
>
> This patch appears to make a number of tests fail. For example:
>
> make check-system-userspace TESTSUITEFLAGS="-v 65 66 74 75 76 77 78 79
> 122"
>
>
> -M
>
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 72999f1ae..aafa6b536 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -1443,7 +1443,7 @@ conntrack_execute(struct conntrack *ct, struct
> > dp_packet_batch *pkt_batch,
> > const struct nat_action_info_t *nat_action_info,
> > long long now, uint32_t tp_id, struct ipf_ctx
> > *ipf_ctx)
> > {
> > - ipf_preprocess_conntrack(ct->ipf, pkt_batch, now, dl_type, zone,
> > + ipf_preprocess_conntrack(ct->ipf, pkt_batch, ipf_ctx, now,
> > dl_type, zone,
> > ct->hash_basis);
> >
> > struct dp_packet *packet;
> > diff --git a/lib/ipf.c b/lib/ipf.c
> > index ef302e59c..bca34f59c 100644
> > --- a/lib/ipf.c
> > +++ b/lib/ipf.c
> > @@ -1139,7 +1139,8 @@ ipf_send_expired_frags(struct ipf *ipf, struct
> > dp_packet_batch *pb,
> > /* Adds a reassmebled packet to a packet batch to be processed by
> > the caller.
> > */
> > static void
> > -ipf_execute_reass_pkts(struct ipf *ipf, struct dp_packet_batch *pb)
> > +ipf_execute_reass_pkts(struct ipf *ipf, struct dp_packet_batch *pb,
> > + struct ipf_ctx *ctx)
> > {
> > if (ovs_list_is_empty(&ipf->reassembled_pkt_list)) {
> > return;
> > @@ -1149,6 +1150,10 @@ ipf_execute_reass_pkts(struct ipf *ipf, struct
> > dp_packet_batch *pb)
> > struct reassembled_pkt *rp, *next;
> >
> > LIST_FOR_EACH_SAFE (rp, next, rp_list_node, &ipf-
> > >reassembled_pkt_list) {
> > + if (ctx && !ipf_ctx_eq(rp->list, ctx, rp->pkt)) {
> > + continue;
> > + }
> > +
> > if (!rp->list->reass_execute_ctx &&
> > ipf_dp_packet_batch_add(pb, rp->pkt, false)) {
> > rp->list->reass_execute_ctx = rp->pkt;
> > @@ -1250,6 +1255,7 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
> > * be added to the batch to be sent through conntrack. */
> > void
> > ipf_preprocess_conntrack(struct ipf *ipf, struct dp_packet_batch
> > *pb,
> > + struct ipf_ctx *ipf_ctx,
> > long long now, ovs_be16 dl_type,
> > uint16_t zone, uint32_t hash_basis)
> > {
> > @@ -1258,7 +1264,7 @@ ipf_preprocess_conntrack(struct ipf *ipf,
> > struct dp_packet_batch *pb,
> > }
> >
> > if (ipf_get_enabled(ipf) || atomic_count_get(&ipf->nfrag)) {
> > - ipf_execute_reass_pkts(ipf, pb);
> > + ipf_execute_reass_pkts(ipf, pb, ipf_ctx);
> > }
> > }
> >
> > diff --git a/lib/ipf.h b/lib/ipf.h
> > index 7bbf453af..8974f15ae 100644
> > --- a/lib/ipf.h
> > +++ b/lib/ipf.h
> > @@ -49,6 +49,7 @@ struct ipf_ctx {
> > struct ipf *ipf_init(void);
> > void ipf_destroy(struct ipf *ipf);
> > void ipf_preprocess_conntrack(struct ipf *ipf, struct
> > dp_packet_batch *pb,
> > + struct ipf_ctx *ctx,
> > long long now, ovs_be16 dl_type,
> > uint16_t zone,
> > uint32_t hash_basis);
> >
>
>
--
hepeng
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
