| 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
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), 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(-)