Hi Numan, Sure, i will submit a v5 soon.
Regards, Ankur ________________________________ From: Numan Siddique <num...@ovn.org> Sent: Thursday, November 21, 2019 3:07 AM To: Ankur Sharma <ankur.sha...@nutanix.com> Cc: ovs-dev@openvswitch.org <ovs-dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v4 0/3] Associate identifier with OVN ACL connection tracking entry On Sat, Nov 9, 2019 at 8:20 AM Ankur Sharma <ankur.sha...@nutanix.com> wrote: > > I submitted this patch long time back and somehow lost track it. > Resubmitting the series, calling it as V4, as it addresses the > review comments given till v3. > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2019-2DApril_358280.html&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=lJy4S1qKbRVivfyfmInKh6pOm1jqpisaSfNyBi90Avg&s=Sa7nACYuFbbSQiI4wVbJgTNMbhoX6bcB-ygpO0vnAvo&e= > > What: > ==== > a. Goal is to be able to associate some identifier with a connection tracking > entry. > > b. This identifier can be used to map OVN ACL which added this entry or > higher level constructs like openstack security group etc. > > c. There are 2 connection tracking fields which can be used for it. > ct.mark (32 bits) and ct.label (128 bits). > > d. Patch intends to use ct.label, as this is a longer field and > hence would be put to a better use, if it stores the identifier. > > Why: > ==== > a. Adding an identifier would help in debugging. > b. Now, we can map a connection tracking entry to corresponding > acl, security group etc. > c. One of the use cases for this mapping would be to identify > ACLs which added corresponding connection tracker entry, which > is causing unexpected drops/leaks. > > How: > ==== > Following is the sequence of changes: > > Patch 1: > i. Current implementation uses a bit ct.label to handle policy update > cases, > where we use a bit in ct.label to indicate that reply traffic should > be dropped now. > ii. Swap the usage of ct.label in current implementation with ct.mark. > > Patch 2: > i. Add support in parser to allow ct.label and mark to be set from > registers > as well (as of now only integer/masked integer is allowed). > > Patch 3: > i. Add a new column (named 'label') to Table ACL in northbound schema. > ii. ovn-northd changes to enhance logical flows to set ct.label to > acl->label. > For example: > table=4 (ls_out_acl ), .... action=(reg0[1] = 1; reg0[3] = 1; > xxreg1 = 0x1234; next;) > . > . > . > table=7 (ls_out_stateful ), ... match=(reg0[1] == 1 && reg0[3] == 1), > > Hi Ankur, Overall the series looks good to me. Can you please rebase and submit v5. In patch 2, using register offsets is not supported. Something like below "ct_commit(ct_label=xreg1[10..30]);" I would suggest to support that case as well. Thanks Numan > Ankur Sharma (3): > OVN ACL: Replace the usage of ct_label with ct_mark > OVN ACL: Allow ct_mark and ct_label values to be set from register as > well > OVN ACL: Allow a user to input ct.label value for an acl > > Documentation/tutorials/ovn-openstack.rst | 14 ++--- > include/ovn/actions.h | 3 + > lib/actions.c | 72 ++++++++++++++++++---- > lib/logical-fields.c | 3 + > northd/ovn-northd.8.xml | 14 ++--- > northd/ovn-northd.c | 87 +++++++++++++++++---------- > ovn-nb.ovsschema | 5 +- > ovn-nb.xml | 12 ++++ > ovn-sb.xml | 41 +++++++++---- > tests/ovn-nbctl.at | 12 +++- > tests/ovn.at | 99 > +++++++++++++++++++++++++++++-- > utilities/ovn-nbctl.c | 24 +++++++- > 12 files changed, 310 insertions(+), 76 deletions(-) > > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=lJy4S1qKbRVivfyfmInKh6pOm1jqpisaSfNyBi90Avg&s=8bIoE5Ex1Np-F8mBWnsmQzn1d0aO61JWUuAKNBHRS0Y&e= _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
