Message ID | 1573267855-102768-1-git-send-email-ankur.sharma@nutanix.com |
---|---|
Headers | show |
Series | Associate identifier with OVN ACL connection tracking entry | expand |
On Sat, Nov 9, 2019 at 8:20 AM Ankur Sharma <ankur.sharma@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://mail.openvswitch.org/pipermail/ovs-dev/2019-April/358280.html > > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Numan, Sure, i will submit a v5 soon. Regards, Ankur