| Message ID | 253303cb8270f453d4bc6b91003548bb8db9889c.1649268700.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] northd: fix nat-v6 with exempted_ext_ips configuration | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | fail | apply and check: fail |
Looks good to me Lorenzo, thanks. Acked-by: Mark Michelson <mmichels@redhat.com> On 4/6/22 14:13, Lorenzo Bianconi wrote: > Use cidr_bits instead of ovs_be32 mask to compute logical flows > priority for nat rules if exempted_ext_ips is specified. > This patch fix an issue for IPv6 snat where exempted_ext_ips > configuration does not take effect. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2066611 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > northd/northd.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 2fb0a93c2..c3a52b714 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -10171,7 +10171,7 @@ static inline void > lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, > struct hmap *lflows, struct ds *match, > const struct nbrec_nat *nat, > - bool is_v6, bool is_src, ovs_be32 mask) > + bool is_v6, bool is_src, int cidr_bits) > { > struct nbrec_address_set *allowed_ext_ips = nat->allowed_ext_ips; > struct nbrec_address_set *exempted_ext_ips = nat->exempted_ext_ips; > @@ -10207,7 +10207,7 @@ lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, > priority = 100 + 2; > } else { > /* S_ROUTER_OUT_SNAT uses priority (mask + 1 + 128 + 1) */ > - priority = count_1bits(ntohl(mask)) + 3; > + priority = cidr_bits + 3; > > if (!od->is_gw_router) { > priority += 128; > @@ -12728,7 +12728,7 @@ static void > build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, > const struct nbrec_nat *nat, struct ds *match, > struct ds *actions, bool distributed, > - ovs_be32 mask, bool is_v6) > + bool is_v6, int cidr_bits) > { > /* Ingress DNAT table: Packets enter the pipeline with destination > * IP address that needs to be DNATted from a external IP address > @@ -12746,7 +12746,7 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, > ds_clear(actions); > if (nat->allowed_ext_ips || nat->exempted_ext_ips) { > lrouter_nat_add_ext_ip_match(od, lflows, match, nat, > - is_v6, true, mask); > + is_v6, true, cidr_bits); > } > > if (!lport_addresses_is_empty(&od->dnat_force_snat_addrs)) { > @@ -12790,7 +12790,7 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, > ds_clear(actions); > if (nat->allowed_ext_ips || nat->exempted_ext_ips) { > lrouter_nat_add_ext_ip_match(od, lflows, match, nat, > - is_v6, true, mask); > + is_v6, true, cidr_bits); > } > > if (!strcmp(nat->type, "dnat_and_snat") && stateless) { > @@ -12893,8 +12893,7 @@ static void > build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > const struct nbrec_nat *nat, struct ds *match, > struct ds *actions, bool distributed, > - struct eth_addr mac, ovs_be32 mask, > - int cidr_bits, bool is_v6) > + struct eth_addr mac, int cidr_bits, bool is_v6) > { > /* Egress SNAT table: Packets enter the egress pipeline with > * source ip address that needs to be SNATted to a external ip > @@ -12912,7 +12911,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > > if (nat->allowed_ext_ips || nat->exempted_ext_ips) { > lrouter_nat_add_ext_ip_match(od, lflows, match, nat, > - is_v6, false, mask); > + is_v6, false, cidr_bits); > } > > if (!strcmp(nat->type, "dnat_and_snat") && stateless) { > @@ -12960,7 +12959,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > > if (nat->allowed_ext_ips || nat->exempted_ext_ips) { > lrouter_nat_add_ext_ip_match(od, lflows, match, nat, > - is_v6, false, mask); > + is_v6, false, cidr_bits); > } > > if (distributed) { > @@ -13305,7 +13304,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, > is_v6); > /* S_ROUTER_IN_DNAT */ > build_lrouter_in_dnat_flow(lflows, od, nat, match, actions, distributed, > - mask, is_v6); > + is_v6, cidr_bits); > > /* ARP resolve for NAT IPs. */ > if (od->is_gw_router) { > @@ -13344,7 +13343,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, > mac, is_v6); > /* S_ROUTER_OUT_SNAT */ > build_lrouter_out_snat_flow(lflows, od, nat, match, actions, distributed, > - mac, mask, cidr_bits, is_v6); > + mac, cidr_bits, is_v6); > > /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ > build_lrouter_ingress_flow(lflows, od, nat, match, actions, >
Hi Lorenzo, I was going through old patches and noticed this one had not been merged, even though it was acked a long time ago. The problem is that it no longer applies cleanly. Can you please rebase this patch and post it again? I'll merge it ASAP. Thanks, Mark On 4/14/22 13:26, Mark Michelson wrote: > Looks good to me Lorenzo, thanks. > > Acked-by: Mark Michelson <mmichels@redhat.com> > > On 4/6/22 14:13, Lorenzo Bianconi wrote: >> Use cidr_bits instead of ovs_be32 mask to compute logical flows >> priority for nat rules if exempted_ext_ips is specified. >> This patch fix an issue for IPv6 snat where exempted_ext_ips >> configuration does not take effect. >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2066611 >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> --- >> northd/northd.c | 21 ++++++++++----------- >> 1 file changed, 10 insertions(+), 11 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 2fb0a93c2..c3a52b714 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -10171,7 +10171,7 @@ static inline void >> lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, >> struct hmap *lflows, struct ds *match, >> const struct nbrec_nat *nat, >> - bool is_v6, bool is_src, ovs_be32 mask) >> + bool is_v6, bool is_src, int cidr_bits) >> { >> struct nbrec_address_set *allowed_ext_ips = nat->allowed_ext_ips; >> struct nbrec_address_set *exempted_ext_ips = nat->exempted_ext_ips; >> @@ -10207,7 +10207,7 @@ lrouter_nat_add_ext_ip_match(struct >> ovn_datapath *od, >> priority = 100 + 2; >> } else { >> /* S_ROUTER_OUT_SNAT uses priority (mask + 1 + 128 + 1) */ >> - priority = count_1bits(ntohl(mask)) + 3; >> + priority = cidr_bits + 3; >> if (!od->is_gw_router) { >> priority += 128; >> @@ -12728,7 +12728,7 @@ static void >> build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath >> *od, >> const struct nbrec_nat *nat, struct ds >> *match, >> struct ds *actions, bool distributed, >> - ovs_be32 mask, bool is_v6) >> + bool is_v6, int cidr_bits) >> { >> /* Ingress DNAT table: Packets enter the pipeline with destination >> * IP address that needs to be DNATted from a external IP address >> @@ -12746,7 +12746,7 @@ build_lrouter_in_dnat_flow(struct hmap >> *lflows, struct ovn_datapath *od, >> ds_clear(actions); >> if (nat->allowed_ext_ips || nat->exempted_ext_ips) { >> lrouter_nat_add_ext_ip_match(od, lflows, match, nat, >> - is_v6, true, mask); >> + is_v6, true, cidr_bits); >> } >> if >> (!lport_addresses_is_empty(&od->dnat_force_snat_addrs)) { >> @@ -12790,7 +12790,7 @@ build_lrouter_in_dnat_flow(struct hmap >> *lflows, struct ovn_datapath *od, >> ds_clear(actions); >> if (nat->allowed_ext_ips || nat->exempted_ext_ips) { >> lrouter_nat_add_ext_ip_match(od, lflows, match, nat, >> - is_v6, true, mask); >> + is_v6, true, cidr_bits); >> } >> if (!strcmp(nat->type, "dnat_and_snat") && stateless) { >> @@ -12893,8 +12893,7 @@ static void >> build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath >> *od, >> const struct nbrec_nat *nat, struct ds >> *match, >> struct ds *actions, bool distributed, >> - struct eth_addr mac, ovs_be32 mask, >> - int cidr_bits, bool is_v6) >> + struct eth_addr mac, int cidr_bits, bool >> is_v6) >> { >> /* Egress SNAT table: Packets enter the egress pipeline with >> * source ip address that needs to be SNATted to a external ip >> @@ -12912,7 +12911,7 @@ build_lrouter_out_snat_flow(struct hmap >> *lflows, struct ovn_datapath *od, >> if (nat->allowed_ext_ips || nat->exempted_ext_ips) { >> lrouter_nat_add_ext_ip_match(od, lflows, match, nat, >> - is_v6, false, mask); >> + is_v6, false, cidr_bits); >> } >> if (!strcmp(nat->type, "dnat_and_snat") && stateless) { >> @@ -12960,7 +12959,7 @@ build_lrouter_out_snat_flow(struct hmap >> *lflows, struct ovn_datapath *od, >> if (nat->allowed_ext_ips || nat->exempted_ext_ips) { >> lrouter_nat_add_ext_ip_match(od, lflows, match, nat, >> - is_v6, false, mask); >> + is_v6, false, cidr_bits); >> } >> if (distributed) { >> @@ -13305,7 +13304,7 @@ build_lrouter_nat_defrag_and_lb(struct >> ovn_datapath *od, struct hmap *lflows, >> is_v6); >> /* S_ROUTER_IN_DNAT */ >> build_lrouter_in_dnat_flow(lflows, od, nat, match, actions, >> distributed, >> - mask, is_v6); >> + is_v6, cidr_bits); >> /* ARP resolve for NAT IPs. */ >> if (od->is_gw_router) { >> @@ -13344,7 +13343,7 @@ build_lrouter_nat_defrag_and_lb(struct >> ovn_datapath *od, struct hmap *lflows, >> mac, is_v6); >> /* S_ROUTER_OUT_SNAT */ >> build_lrouter_out_snat_flow(lflows, od, nat, match, actions, >> distributed, >> - mac, mask, cidr_bits, is_v6); >> + mac, cidr_bits, is_v6); >> /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ >> build_lrouter_ingress_flow(lflows, od, nat, match, actions, >> >
diff --git a/northd/northd.c b/northd/northd.c index 2fb0a93c2..c3a52b714 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -10171,7 +10171,7 @@ static inline void lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, struct hmap *lflows, struct ds *match, const struct nbrec_nat *nat, - bool is_v6, bool is_src, ovs_be32 mask) + bool is_v6, bool is_src, int cidr_bits) { struct nbrec_address_set *allowed_ext_ips = nat->allowed_ext_ips; struct nbrec_address_set *exempted_ext_ips = nat->exempted_ext_ips; @@ -10207,7 +10207,7 @@ lrouter_nat_add_ext_ip_match(struct ovn_datapath *od, priority = 100 + 2; } else { /* S_ROUTER_OUT_SNAT uses priority (mask + 1 + 128 + 1) */ - priority = count_1bits(ntohl(mask)) + 3; + priority = cidr_bits + 3; if (!od->is_gw_router) { priority += 128; @@ -12728,7 +12728,7 @@ static void build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, - ovs_be32 mask, bool is_v6) + bool is_v6, int cidr_bits) { /* Ingress DNAT table: Packets enter the pipeline with destination * IP address that needs to be DNATted from a external IP address @@ -12746,7 +12746,7 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, ds_clear(actions); if (nat->allowed_ext_ips || nat->exempted_ext_ips) { lrouter_nat_add_ext_ip_match(od, lflows, match, nat, - is_v6, true, mask); + is_v6, true, cidr_bits); } if (!lport_addresses_is_empty(&od->dnat_force_snat_addrs)) { @@ -12790,7 +12790,7 @@ build_lrouter_in_dnat_flow(struct hmap *lflows, struct ovn_datapath *od, ds_clear(actions); if (nat->allowed_ext_ips || nat->exempted_ext_ips) { lrouter_nat_add_ext_ip_match(od, lflows, match, nat, - is_v6, true, mask); + is_v6, true, cidr_bits); } if (!strcmp(nat->type, "dnat_and_snat") && stateless) { @@ -12893,8 +12893,7 @@ static void build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, const struct nbrec_nat *nat, struct ds *match, struct ds *actions, bool distributed, - struct eth_addr mac, ovs_be32 mask, - int cidr_bits, bool is_v6) + struct eth_addr mac, int cidr_bits, bool is_v6) { /* Egress SNAT table: Packets enter the egress pipeline with * source ip address that needs to be SNATted to a external ip @@ -12912,7 +12911,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, if (nat->allowed_ext_ips || nat->exempted_ext_ips) { lrouter_nat_add_ext_ip_match(od, lflows, match, nat, - is_v6, false, mask); + is_v6, false, cidr_bits); } if (!strcmp(nat->type, "dnat_and_snat") && stateless) { @@ -12960,7 +12959,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, if (nat->allowed_ext_ips || nat->exempted_ext_ips) { lrouter_nat_add_ext_ip_match(od, lflows, match, nat, - is_v6, false, mask); + is_v6, false, cidr_bits); } if (distributed) { @@ -13305,7 +13304,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, is_v6); /* S_ROUTER_IN_DNAT */ build_lrouter_in_dnat_flow(lflows, od, nat, match, actions, distributed, - mask, is_v6); + is_v6, cidr_bits); /* ARP resolve for NAT IPs. */ if (od->is_gw_router) { @@ -13344,7 +13343,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, mac, is_v6); /* S_ROUTER_OUT_SNAT */ build_lrouter_out_snat_flow(lflows, od, nat, match, actions, distributed, - mac, mask, cidr_bits, is_v6); + mac, cidr_bits, is_v6); /* S_ROUTER_IN_ADMISSION - S_ROUTER_IN_IP_INPUT */ build_lrouter_ingress_flow(lflows, od, nat, match, actions,
Use cidr_bits instead of ovs_be32 mask to compute logical flows priority for nat rules if exempted_ext_ips is specified. This patch fix an issue for IPv6 snat where exempted_ext_ips configuration does not take effect. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2066611 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- northd/northd.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)