[ovs-dev] [PATCH ovn 0/4] Reduce number of flows in IN_IP_INPUT table for DNAT.
dceara at redhat.com
Fri Jun 26 13:31:41 UTC 2020
On 6/26/20 3:05 PM, Numan Siddique wrote:
> On Wed, Jun 24, 2020 at 9:21 PM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
> The first three patches refactor the ARP/NS responder code for logical
> routers in order to make it easier for patch #4 to configure the flows
> with different priorities depending on logical port type.
> Suggested-by: Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>>
> Reported-by: Girish Moodalbail <gmoodalbail at gmail.com
> <mailto:gmoodalbail at gmail.com>>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>>
> Dumitru Ceara (4):
> ovn-northd: Store ETH address of router inport in xxreg0.
> ovn-northd: Refactor ARP/NS responder in router pipeline.
> ovn-northd: Refactor NAT address parsing.
> ovn-northd: Minimize number of ARP/NS responder flows for DNAT.
> Hi Dumitru,
> Thanks for this patch series.
> I didn't review them thoroughly. I've few comments.
Thanks for looking at these patches.
> 1. Looks like reg0 and xxreg0 are already used in the router pipeline. I
> think it's better to use a different register (xxreg1 may be).
Yes, xxreg0 is already used in the router pipeline, but only in later
stages. However, I'll use xxreg1 instead just to make sure there's no
> I'd suggest adding a new macro REG_IP_ROUTING (or something more
> meaningful) instead of using reg0/xxreg0 in the existing
> code in the lr_in_ip_input/lr_in_arp_resolve stages.
I'm not really sure I follow. I added a macro:
#define REG_INPORT_ETH_ADDR "xxreg0[80..127]"
And there's no bare reference to xxreg0 in my patches, I always used the
macro. Did I miss anything?
$ git diff origin/master northd/ovn-northd.c | grep xxreg
+#define REG_INPORT_ETH_ADDR "xxreg0[80..127]"
> 2. Does patch 2 require updating the ovn-northd.8.xml ?
It shouldn't have to because it's just a refactoring that doesn't change
the way flows are generated.
> 3. For patches which modify the logical flows, can you please add a few
> test cases in ovn-northd.at <http://ovn-northd.at>. That would also help
> me while
> reviewing the code :)
Sure, will do in v2.
> northd/ovn-northd.8.xml | 38 ++-
> northd/ovn-northd.c | 565
> tests/ovn.at <http://ovn.at> | 8 -
> 3 files changed, 361 insertions(+), 250 deletions(-)
> dev mailing list
> dev at openvswitch.org <mailto:dev at openvswitch.org>
More information about the dev