Re: [ovs-dev] [PATCH ovn 6/7] ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.
On Thu, Jul 30, 2020 at 12:18 PM Han Zhou wrote:
> On Wed, Jul 29, 2020 at 10:37 AM Numan Siddique wrote:
> >
> >
> >
> > On Thu, Jul 23, 2020 at 10:57 AM Han Zhou wrote:
> >>
> >> In LR ingress stage LOOKUP_NEIGHBOR and LEARN_NEIGHBOR, the flag
> >> REGBIT_SKIP_LOOKUP_NEIGHBOR was used to indicate if mac-binding
> >> lookup can be skipped. This patch avoid using the bit by combining
> >> it with the REGBIT_LOOKUP_NEIGHBOR_RESULT bit, and assigning 1
> >> to REGBIT_LOOKUP_NEIGHBOR_RESULT serves same purpose of skipping
> >> the lookup. There will be a new bit needed in a future patch, and
> >> this change can avoid using too many bits unnecessarily.
> >>
> >> Signed-off-by: Han Zhou
> >
> >
> > Acked-by: Numan Siddique
> >
> > Thanks
> > Numan
> >
>
> Thanks Numan for the review. I applied the patches 2, 3, 5, 6 of this
> series, which you have already acked, to master. It will simplify the work
> for v2. I will update the rest patches and send v2 after I get all your
> feedback.
>
Hi Han,
The other patches look good to me. I don't have any comments other than
what I provided earlier - Having test cases in ovn-northd.at will be
helpful so that there
are no regressions later.
Thanks
Numan
>
> Thanks,
> Han
>
> >>
> >> ---
> >> northd/ovn-northd.8.xml | 11 ---
> >> northd/ovn-northd.c | 6 ++
> >> 2 files changed, 6 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> >> index 741c928..9f2c70f 100644
> >> --- a/northd/ovn-northd.8.xml
> >> +++ b/northd/ovn-northd.8.xml
> >> @@ -1595,7 +1595,7 @@ next;
> >>
> >>
> >> A priority-0 fallback flow that matches all packets and applies
> >> -the action reg9[3] = 1; next;
> >> +the action reg9[2] = 1; next;
> >> advancing the packet to the next table.
> >>
> >>
> >> @@ -1610,17 +1610,14 @@ next;
> >>
> >>
> >>reg9[2] will be 1 if the
> lookup_arp/lookup_nd
> >> - in the previous table was successful.
> >> -
> >> -
> >> -
> >> - reg9[3] will be 1 if there was no need to do the
> lookup.
> >> + in the previous table was successful, or if there was no need to
> do the
> >> + lookup.
> >>
> >>
> >>
> >>
> >> A priority-100 flow with the match
> >> -reg9[2] == 1 || reg9[3] == 1 and advances the
> packet
> >> +reg9[2] == 1 and advances the packet
> >> to the next table as there is no need to learn the neighbor.
> >>
> >>
> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> >> index 4e11a25..425f522 100644
> >> --- a/northd/ovn-northd.c
> >> +++ b/northd/ovn-northd.c
> >> @@ -221,7 +221,6 @@ enum ovn_stage {
> >> /* Register to store the result of check_pkt_larger action. */
> >> #define REGBIT_PKT_LARGER"reg9[1]"
> >> #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
> >> -#define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
> >>
> >> /* Register to store the eth address associated to a router port for
> packets
> >> * received in S_ROUTER_IN_ADMISSION.
> >> @@ -8275,14 +8274,13 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> >>"lookup_nd(inport, ip6.src, nd.sll); next;");
> >>
> >> /* For other packet types, we can skip neighbor learning.
> >> - * So set REGBIT_SKIP_LOOKUP_NEIGHBOR to 1. */
> >> + * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
> >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
> >> - REGBIT_SKIP_LOOKUP_NEIGHBOR" = 1; next;");
> >> + REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;");
> >>
> >> /* Flows for LEARN_NEIGHBOR. */
> >> /* Skip Neighbor learning if not required. */
> >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
> >> - REGBIT_SKIP_LOOKUP_NEIGHBOR" == 1 || "
> >>REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
> >>
> >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
> >> --
> >> 2.1.0
> >>
> >> ___
> >> dev mailing list
> >> d...@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 6/7] ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.
On Wed, Jul 29, 2020 at 10:37 AM Numan Siddique wrote:
>
>
>
> On Thu, Jul 23, 2020 at 10:57 AM Han Zhou wrote:
>>
>> In LR ingress stage LOOKUP_NEIGHBOR and LEARN_NEIGHBOR, the flag
>> REGBIT_SKIP_LOOKUP_NEIGHBOR was used to indicate if mac-binding
>> lookup can be skipped. This patch avoid using the bit by combining
>> it with the REGBIT_LOOKUP_NEIGHBOR_RESULT bit, and assigning 1
>> to REGBIT_LOOKUP_NEIGHBOR_RESULT serves same purpose of skipping
>> the lookup. There will be a new bit needed in a future patch, and
>> this change can avoid using too many bits unnecessarily.
>>
>> Signed-off-by: Han Zhou
>
>
> Acked-by: Numan Siddique
>
> Thanks
> Numan
>
Thanks Numan for the review. I applied the patches 2, 3, 5, 6 of this
series, which you have already acked, to master. It will simplify the work
for v2. I will update the rest patches and send v2 after I get all your
feedback.
Thanks,
Han
>>
>> ---
>> northd/ovn-northd.8.xml | 11 ---
>> northd/ovn-northd.c | 6 ++
>> 2 files changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>> index 741c928..9f2c70f 100644
>> --- a/northd/ovn-northd.8.xml
>> +++ b/northd/ovn-northd.8.xml
>> @@ -1595,7 +1595,7 @@ next;
>>
>>
>> A priority-0 fallback flow that matches all packets and applies
>> -the action reg9[3] = 1; next;
>> +the action reg9[2] = 1; next;
>> advancing the packet to the next table.
>>
>>
>> @@ -1610,17 +1610,14 @@ next;
>>
>>
>>reg9[2] will be 1 if the
lookup_arp/lookup_nd
>> - in the previous table was successful.
>> -
>> -
>> -
>> - reg9[3] will be 1 if there was no need to do the
lookup.
>> + in the previous table was successful, or if there was no need to
do the
>> + lookup.
>>
>>
>>
>>
>> A priority-100 flow with the match
>> -reg9[2] == 1 || reg9[3] == 1 and advances the
packet
>> +reg9[2] == 1 and advances the packet
>> to the next table as there is no need to learn the neighbor.
>>
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index 4e11a25..425f522 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -221,7 +221,6 @@ enum ovn_stage {
>> /* Register to store the result of check_pkt_larger action. */
>> #define REGBIT_PKT_LARGER"reg9[1]"
>> #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
>> -#define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
>>
>> /* Register to store the eth address associated to a router port for
packets
>> * received in S_ROUTER_IN_ADMISSION.
>> @@ -8275,14 +8274,13 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
>>"lookup_nd(inport, ip6.src, nd.sll); next;");
>>
>> /* For other packet types, we can skip neighbor learning.
>> - * So set REGBIT_SKIP_LOOKUP_NEIGHBOR to 1. */
>> + * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
>> - REGBIT_SKIP_LOOKUP_NEIGHBOR" = 1; next;");
>> + REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;");
>>
>> /* Flows for LEARN_NEIGHBOR. */
>> /* Skip Neighbor learning if not required. */
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
>> - REGBIT_SKIP_LOOKUP_NEIGHBOR" == 1 || "
>>REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
>>
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
>> --
>> 2.1.0
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 6/7] ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.
On Thu, Jul 23, 2020 at 10:57 AM Han Zhou wrote:
> In LR ingress stage LOOKUP_NEIGHBOR and LEARN_NEIGHBOR, the flag
> REGBIT_SKIP_LOOKUP_NEIGHBOR was used to indicate if mac-binding
> lookup can be skipped. This patch avoid using the bit by combining
> it with the REGBIT_LOOKUP_NEIGHBOR_RESULT bit, and assigning 1
> to REGBIT_LOOKUP_NEIGHBOR_RESULT serves same purpose of skipping
> the lookup. There will be a new bit needed in a future patch, and
> this change can avoid using too many bits unnecessarily.
>
> Signed-off-by: Han Zhou
>
Acked-by: Numan Siddique
Thanks
Numan
> ---
> northd/ovn-northd.8.xml | 11 ---
> northd/ovn-northd.c | 6 ++
> 2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 741c928..9f2c70f 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1595,7 +1595,7 @@ next;
>
>
> A priority-0 fallback flow that matches all packets and applies
> -the action reg9[3] = 1; next;
> +the action reg9[2] = 1; next;
> advancing the packet to the next table.
>
>
> @@ -1610,17 +1610,14 @@ next;
>
>
>reg9[2] will be 1 if the
> lookup_arp/lookup_nd
> - in the previous table was successful.
> -
> -
> -
> - reg9[3] will be 1 if there was no need to do the
> lookup.
> + in the previous table was successful, or if there was no need to do
> the
> + lookup.
>
>
>
>
> A priority-100 flow with the match
> -reg9[2] == 1 || reg9[3] == 1 and advances the packet
> +reg9[2] == 1 and advances the packet
> to the next table as there is no need to learn the neighbor.
>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4e11a25..425f522 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -221,7 +221,6 @@ enum ovn_stage {
> /* Register to store the result of check_pkt_larger action. */
> #define REGBIT_PKT_LARGER"reg9[1]"
> #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
> -#define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
>
> /* Register to store the eth address associated to a router port for
> packets
> * received in S_ROUTER_IN_ADMISSION.
> @@ -8275,14 +8274,13 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>"lookup_nd(inport, ip6.src, nd.sll); next;");
>
> /* For other packet types, we can skip neighbor learning.
> - * So set REGBIT_SKIP_LOOKUP_NEIGHBOR to 1. */
> + * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
> - REGBIT_SKIP_LOOKUP_NEIGHBOR" = 1; next;");
> + REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;");
>
> /* Flows for LEARN_NEIGHBOR. */
> /* Skip Neighbor learning if not required. */
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
> - REGBIT_SKIP_LOOKUP_NEIGHBOR" == 1 || "
>REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
>
> ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
> --
> 2.1.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 6/7] ovn-northd.c: Remove the use of the REGBIT_SKIP_LOOKUP_NEIGHBOR bit.
In LR ingress stage LOOKUP_NEIGHBOR and LEARN_NEIGHBOR, the flag
REGBIT_SKIP_LOOKUP_NEIGHBOR was used to indicate if mac-binding
lookup can be skipped. This patch avoid using the bit by combining
it with the REGBIT_LOOKUP_NEIGHBOR_RESULT bit, and assigning 1
to REGBIT_LOOKUP_NEIGHBOR_RESULT serves same purpose of skipping
the lookup. There will be a new bit needed in a future patch, and
this change can avoid using too many bits unnecessarily.
Signed-off-by: Han Zhou
---
northd/ovn-northd.8.xml | 11 ---
northd/ovn-northd.c | 6 ++
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 741c928..9f2c70f 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1595,7 +1595,7 @@ next;
A priority-0 fallback flow that matches all packets and applies
-the action reg9[3] = 1; next;
+the action reg9[2] = 1; next;
advancing the packet to the next table.
@@ -1610,17 +1610,14 @@ next;
reg9[2] will be 1 if the lookup_arp/lookup_nd
- in the previous table was successful.
-
-
-
- reg9[3] will be 1 if there was no need to do the lookup.
+ in the previous table was successful, or if there was no need to do the
+ lookup.
A priority-100 flow with the match
-reg9[2] == 1 || reg9[3] == 1 and advances the packet
+reg9[2] == 1 and advances the packet
to the next table as there is no need to learn the neighbor.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 4e11a25..425f522 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -221,7 +221,6 @@ enum ovn_stage {
/* Register to store the result of check_pkt_larger action. */
#define REGBIT_PKT_LARGER"reg9[1]"
#define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
-#define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
/* Register to store the eth address associated to a router port for packets
* received in S_ROUTER_IN_ADMISSION.
@@ -8275,14 +8274,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
"lookup_nd(inport, ip6.src, nd.sll); next;");
/* For other packet types, we can skip neighbor learning.
- * So set REGBIT_SKIP_LOOKUP_NEIGHBOR to 1. */
+ * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */
ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1",
- REGBIT_SKIP_LOOKUP_NEIGHBOR" = 1; next;");
+ REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;");
/* Flows for LEARN_NEIGHBOR. */
/* Skip Neighbor learning if not required. */
ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 100,
- REGBIT_SKIP_LOOKUP_NEIGHBOR" == 1 || "
REGBIT_LOOKUP_NEIGHBOR_RESULT" == 1", "next;");
ovn_lflow_add(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR, 90,
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
