just forget to reply all.
---------- Forwarded message ---------
发件人： 贺鹏 <xnhp0...@gmail.com>
Date: 2022年1月9日周日 10:59
Subject: Re: [ovs-dev v6 3/3] ipf: ipf_preprocess_conntrack should also
consider ipf_ctx
To: Mike Pattrick <m...@redhat.com>
Hi, Mike,
thank you for the review and testing.
it seems that all these broken tests are related to a bug in packet-out
action execution in OVS.
it seems that the packet-out uses the OFP to fill the dp_packet metadata,
and this in_port value has
not been changed during the execution in the datapath, where the value
should be translated into the ODP.
in the test the ofp values equals to 1, while the corresponding odp port
equals to 2, which makes ipf_ctx_eq test failed.
I am trying to think about a solution.
any idaes?
Mike Pattrick <m...@redhat.com> 于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 <hepeng.0...@bytedance.com>
> > ---
> > 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
--
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
