[ovs-dev] [patch_v8] ovn: Fix receive from vxlan in ovn-controller.
Justin Pettit
jpettit at ovn.org
Wed Aug 10 23:33:00 UTC 2016
> On Aug 8, 2016, at 7:20 PM, Darrell Ball <dlu998 at gmail.com> wrote:
>
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 589b053..43885fd 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -496,6 +496,21 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
> ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
> ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100,
> &match, ofpacts_p, &binding->header_.uuid);
> +
> + /* For packets received from a Vxlan tunnel which get
> + * resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to lack of
> + * needed metadata in Vxlan, explicitly skip sending back out
> + * any tunnels and resubmit to table 33 for local delivery. */
The capitalization of "VXLAN" is inconsistent in this comment. I would just use "VXLAN", since that's how it is in most of the code base.
I'd move this comment to the big comment describing "Table 32, priority 100", and adding this priority to that heading.
> + match_init_catchall(&match);
> + ofpbuf_clear(ofpacts_p);
> + match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
> + MLF_RCV_FROM_VXLAN,
> + MLF_RCV_FROM_VXLAN);
> + /* Resubmit to table 33. */
> + put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> + ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 101, &match, ofpacts_p,
> + &binding->header_.uuid);
This code is indented to 9 columns instead of 8.
It seems like we've usually used a priority of 150 for this higher priority flow.
> diff --git a/ovn/lib/logical-fields.h b/ovn/lib/logical-fields.h
> index 1ea2e0f..1053c86 100644
> --- a/ovn/lib/logical-fields.h
> +++ b/ovn/lib/logical-fields.h
>
> @@ -52,4 +43,21 @@ enum {
>
> void ovn_init_symtab(struct shash *symtab);
>
> +/* MFF_LOG_FLAGS_REG bit assignments */
> +enum mff_log_flags_bits {
> + MLF_ALLOW_LOOPBACK_BIT = 0,
> + MLF_RCV_FROM_VXLAN_BIT = 1,
> +};
> +
> +/* MFF_LOG_FLAGS_REG flag assignments */
> +enum mff_log_flags {
> + MLF_ALLOW_LOOPBACK = (1 << MLF_ALLOW_LOOPBACK_BIT), /* Allow outputting
> + back to inport. */
> + /* The below flag is used to indicate that a packet was received from a
> + * vxlan tunnel to compensate for the lack of egress port information
> + * available in Vxlan encapsulation. Egress port information is
> + * available for Geneve and STT tunnel types. */
> + MLF_RCV_FROM_VXLAN = (1 << MLF_RCV_FROM_VXLAN_BIT),
The capitalization of "VXLAN" is inconsistent in this comment, too. I'd also change the comment describing MLF_ALLOW_LOOPBACK to be right above the definition so that it's consistent with the big comment you've added.
I've attached a diff that contains my suggestions (along with a couple other minor things). If you're happy with it, I'll commit the patch.
Thanks,
--Justin
-=-=-=-=-=-=-=-=-
diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
index b2086ad..a72b149 100644
--- a/ovn/controller-vtep/vtep.c
+++ b/ovn/controller-vtep/vtep.c
@@ -231,6 +231,7 @@ vtep_lswitch_run(struct shash *vtep_pbs, struct sset *vtep_pswi
vtep_ls->tunnel_key[0], tnl_key);
}
vteprec_logical_switch_set_tunnel_key(vtep_ls, &tnl_key, 1);
+
/* OVN is expected to always use source node replication mode,
* hence the replication mode is hard-coded for each logical
* switch in the context of ovn-controller-vtep. */
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 43885fd..4448756 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -474,13 +474,31 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
&match, ofpacts_p, &binding->header_.uuid);
} else {
/* Remote port connected by tunnel */
- /* Table 32, priority 100.
- * =======================
+
+ /* Table 32, priority 150 and 100.
+ * ===============================
*
- * Implements output to remote hypervisors. Each flow matches an
- * output port that includes a logical port on a remote hypervisor,
- * and tunnels the packet to that hypervisor.
+ * Priority 150 is for packets received from a VXLAN tunnel
+ * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
+ * lack of needed metadata in VXLAN, explicitly skip sending
+ * back out any tunnels and resubmit to table 33 for local
+ * delivery.
+ *
+ * Priority 100 is for all other traffic which need to be sent
+ * to a remote hypervisor. Each flow matches an output port
+ * that includes a logical port on a remote hypervisor, and
+ * tunnels the packet to that hypervisor.
*/
+ match_init_catchall(&match);
+ ofpbuf_clear(ofpacts_p);
+ match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+ MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
+
+ /* Resubmit to table 33. */
+ put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
+ ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 150, &match, ofpacts_p,
+ &binding->header_.uuid);
+
match_init_catchall(&match);
ofpbuf_clear(ofpacts_p);
@@ -496,21 +514,6 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100,
&match, ofpacts_p, &binding->header_.uuid);
-
- /* For packets received from a Vxlan tunnel which get
- * resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to lack of
- * needed metadata in Vxlan, explicitly skip sending back out
- * any tunnels and resubmit to table 33 for local delivery. */
- match_init_catchall(&match);
- ofpbuf_clear(ofpacts_p);
- match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
- MLF_RCV_FROM_VXLAN,
- MLF_RCV_FROM_VXLAN);
- /* Resubmit to table 33. */
- put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
- ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 101, &match, ofpacts_p,
- &binding->header_.uuid);
-
}
}
diff --git a/ovn/lib/logical-fields.h b/ovn/lib/logical-fields.h
index 1053c86..a1f1da6 100644
--- a/ovn/lib/logical-fields.h
+++ b/ovn/lib/logical-fields.h
@@ -51,12 +51,13 @@ enum mff_log_flags_bits {
/* MFF_LOG_FLAGS_REG flag assignments */
enum mff_log_flags {
- MLF_ALLOW_LOOPBACK = (1 << MLF_ALLOW_LOOPBACK_BIT), /* Allow outputting
- back to inport. */
- /* The below flag is used to indicate that a packet was received from a
- * vxlan tunnel to compensate for the lack of egress port information
- * available in Vxlan encapsulation. Egress port information is
- * available for Geneve and STT tunnel types. */
+ /* Allow outputting back to inport. */
+ MLF_ALLOW_LOOPBACK = (1 << MLF_ALLOW_LOOPBACK_BIT),
+
+ /* Indicate that a packet was received from a VXLAN tunnel to
+ * compensate for the lack of egress port information available in
+ * VXLAN encapsulation. Egress port information is available for
+ * Geneve and STT tunnel types. */
MLF_RCV_FROM_VXLAN = (1 << MLF_RCV_FROM_VXLAN_BIT),
};
More information about the dev
mailing list