diff mbox

[ovs-dev] bfd: Allow setting OAM bit when encapsulated in tunnel.

Message ID 1467178123-33167-1-git-send-email-jesse@kernel.org
State Accepted
Headers show

Commit Message

Jesse Gross June 29, 2016, 5:28 a.m. UTC 
Some tunnel protocols, such as Geneve, have a bit in the tunnel
header to indicate that it is an OAM packet. This means that the
packet should be processed as a tunnel control frame and not be
passed onto connected links.

When BFD is used inside of a tunnel it is often used in this control
capacity, so this adds an option to enable marking the outer header
when the output port is a tunnel that supports the OAM concept. It is
also possible to use tunnels as point-to-point links that are simply
carrying BFD as payload, so this is not always turned on.

Conceptually, this may also apply to other types of packets locally
generated by the switch, most obviously CFM. However, BFD seems to
be most commonly used for this type of tunnel monitoring application
so this only adds the option to BFD for the time being to avoid
unnecessarily adding configuration knobs that might never get used.

Signed-off-by: Jesse Gross <jesse@kernel.org>
---
 lib/bfd.c                      |  7 ++++++-
 lib/bfd.h                      |  2 +-
 ofproto/ofproto-dpif-monitor.c | 10 ++++++----
 ofproto/ofproto-dpif-xlate.c   | 26 ++++++++++++++++++--------
 ofproto/ofproto-dpif-xlate.h   |  2 +-
 ofproto/ofproto-dpif.c         | 16 +++++++++-------
 ofproto/ofproto-dpif.h         |  3 ++-
 vswitchd/vswitch.xml           |  7 +++++++
 8 files changed, 50 insertions(+), 23 deletions(-)

Comments

Pravin Shelar June 29, 2016, 9:41 p.m. UTC | #1 
On Tue, Jun 28, 2016 at 10:28 PM, Jesse Gross <jesse@kernel.org> wrote:
> Some tunnel protocols, such as Geneve, have a bit in the tunnel
> header to indicate that it is an OAM packet. This means that the
> packet should be processed as a tunnel control frame and not be
> passed onto connected links.
>
> When BFD is used inside of a tunnel it is often used in this control
> capacity, so this adds an option to enable marking the outer header
> when the output port is a tunnel that supports the OAM concept. It is
> also possible to use tunnels as point-to-point links that are simply
> carrying BFD as payload, so this is not always turned on.
>
> Conceptually, this may also apply to other types of packets locally
> generated by the switch, most obviously CFM. However, BFD seems to
> be most commonly used for this type of tunnel monitoring application
> so this only adds the option to BFD for the time being to avoid
> unnecessarily adding configuration knobs that might never get used.
>
> Signed-off-by: Jesse Gross <jesse@kernel.org>
> ---
I got white space warning from git am:-

Applying: bfd: Allow setting OAM bit when encapsulated in tunnel.

/home/pravin/ovs/ovs/w8/.git/rebase-apply/patch:210: trailing whitespace.

ofproto_dpif_send_packet(const struct ofport_dpif *ofport, bool oam,

warning: 1 line adds whitespace errors.

---
Otherwise looks good.

Acked-by: Pravin B Shelar <pshelar@ovn.org>
Jesse Gross June 29, 2016, 10:29 p.m. UTC | #2 
On Wed, Jun 29, 2016 at 2:41 PM, pravin shelar <pshelar@ovn.org> wrote:
> On Tue, Jun 28, 2016 at 10:28 PM, Jesse Gross <jesse@kernel.org> wrote:
>> Some tunnel protocols, such as Geneve, have a bit in the tunnel
>> header to indicate that it is an OAM packet. This means that the
>> packet should be processed as a tunnel control frame and not be
>> passed onto connected links.
>>
>> When BFD is used inside of a tunnel it is often used in this control
>> capacity, so this adds an option to enable marking the outer header
>> when the output port is a tunnel that supports the OAM concept. It is
>> also possible to use tunnels as point-to-point links that are simply
>> carrying BFD as payload, so this is not always turned on.
>>
>> Conceptually, this may also apply to other types of packets locally
>> generated by the switch, most obviously CFM. However, BFD seems to
>> be most commonly used for this type of tunnel monitoring application
>> so this only adds the option to BFD for the time being to avoid
>> unnecessarily adding configuration knobs that might never get used.
>>
>> Signed-off-by: Jesse Gross <jesse@kernel.org>
>> ---
> I got white space warning from git am:-
>
> Applying: bfd: Allow setting OAM bit when encapsulated in tunnel.
>
> /home/pravin/ovs/ovs/w8/.git/rebase-apply/patch:210: trailing whitespace.
>
> ofproto_dpif_send_packet(const struct ofport_dpif *ofport, bool oam,
>
> warning: 1 line adds whitespace errors.
>
> ---
> Otherwise looks good.
>
> Acked-by: Pravin B Shelar <pshelar@ovn.org>

Thanks, I fixed the whitespace error and applied this to master.
diff mbox

Patch

diff --git a/lib/bfd.c b/lib/bfd.c
index b40c022..9616c90 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -168,6 +168,8 @@  struct bfd {
     enum flags flags;             /* Flags sent on messages. */
     enum flags rmt_flags;         /* Flags last received. */
 
+    bool oam;                     /* Set tunnel OAM flag if applicable. */
+
     uint32_t rmt_disc;            /* bfd.RemoteDiscr. */
 
     struct eth_addr local_eth_src; /* Local eth src address. */
@@ -390,6 +392,8 @@  bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
         bfd_status_changed(bfd);
     }
 
+    bfd->oam = smap_get_bool(cfg, "oam", false);
+
     atomic_store_relaxed(&bfd->check_tnl_key,
                          smap_get_bool(cfg, "check_tnl_key", false));
     min_tx = smap_get_int(cfg, "min_tx", 100);
@@ -586,7 +590,7 @@  bfd_should_send_packet(const struct bfd *bfd) OVS_EXCLUDED(mutex)
 
 void
 bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
-               const struct eth_addr eth_src) OVS_EXCLUDED(mutex)
+               const struct eth_addr eth_src, bool *oam) OVS_EXCLUDED(mutex)
 {
     long long int min_tx, min_rx;
     struct udp_header *udp;
@@ -654,6 +658,7 @@  bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
     msg->min_rx = htonl(min_rx * 1000);
 
     bfd->flags &= ~FLAG_FINAL;
+    *oam = bfd->oam;
 
     log_msg(VLL_DBG, msg, "Sending BFD Message", bfd);
 
diff --git a/lib/bfd.h b/lib/bfd.h
index 6a1d547..9d32327 100644
--- a/lib/bfd.h
+++ b/lib/bfd.h
@@ -36,7 +36,7 @@  void bfd_run(struct bfd *);
 
 bool bfd_should_send_packet(const struct bfd *);
 void bfd_put_packet(struct bfd *bfd, struct dp_packet *packet,
-                    const struct eth_addr eth_src);
+                    const struct eth_addr eth_src, bool *oam);
 
 bool bfd_should_process_flow(const struct bfd *, const struct flow *,
                              struct flow_wildcards *);
diff --git a/ofproto/ofproto-dpif-monitor.c b/ofproto/ofproto-dpif-monitor.c
index ace4e73..11d7a54 100644
--- a/ofproto/ofproto-dpif-monitor.c
+++ b/ofproto/ofproto-dpif-monitor.c
@@ -277,17 +277,19 @@  monitor_mport_run(struct mport *mport, struct dp_packet *packet)
     if (mport->cfm && cfm_should_send_ccm(mport->cfm)) {
         dp_packet_clear(packet);
         cfm_compose_ccm(mport->cfm, packet, mport->hw_addr);
-        ofproto_dpif_send_packet(mport->ofport, packet);
+        ofproto_dpif_send_packet(mport->ofport, false, packet);
     }
     if (mport->bfd && bfd_should_send_packet(mport->bfd)) {
+        bool oam;
+
         dp_packet_clear(packet);
-        bfd_put_packet(mport->bfd, packet, mport->hw_addr);
-        ofproto_dpif_send_packet(mport->ofport, packet);
+        bfd_put_packet(mport->bfd, packet, mport->hw_addr, &oam);
+        ofproto_dpif_send_packet(mport->ofport, oam, packet);
     }
     if (mport->lldp && lldp_should_send_packet(mport->lldp)) {
         dp_packet_clear(packet);
         lldp_put_packet(mport->lldp, packet, mport->hw_addr);
-        ofproto_dpif_send_packet(mport->ofport, packet);
+        ofproto_dpif_send_packet(mport->ofport, false, packet);
     }
 
     if (mport->cfm) {
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d46a52c..e627760 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5724,18 +5724,21 @@  xlate_resume(struct ofproto_dpif *ofproto,
     return error == XLATE_BRIDGE_NOT_FOUND ? OFPERR_NXR_STALE : 0;
 }
 
-/* Sends 'packet' out 'ofport'.
+/* Sends 'packet' out 'ofport'. If 'port' is a tunnel and that tunnel type
+ * supports a notion of an OAM flag, sets it if 'oam' is true.
  * May modify 'packet'.
  * Returns 0 if successful, otherwise a positive errno value. */
 int
-xlate_send_packet(const struct ofport_dpif *ofport, struct dp_packet *packet)
+xlate_send_packet(const struct ofport_dpif *ofport, bool oam,
+                  struct dp_packet *packet)
 {
     struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
     struct xport *xport;
-    struct ofpact_output output;
+    uint64_t ofpacts_stub[1024 / 8];
+    struct ofpbuf ofpacts;
     struct flow flow;
 
-    ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
+    ofpbuf_use_stack(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
     /* Use OFPP_NONE as the in_port to avoid special packet processing. */
     flow_extract(packet, &flow);
     flow.in_port.ofp_port = OFPP_NONE;
@@ -5744,12 +5747,19 @@  xlate_send_packet(const struct ofport_dpif *ofport, struct dp_packet *packet)
     if (!xport) {
         return EINVAL;
     }
-    output.port = xport->ofp_port;
-    output.max_len = 0;
+
+    if (oam) {
+        struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
+
+        sf->field = mf_from_id(MFF_TUN_FLAGS);
+        sf->value.be16 = htons(NX_TUN_FLAG_OAM);
+        sf->mask.be16 = htons(NX_TUN_FLAG_OAM);
+    }
+
+    ofpact_put_OUTPUT(&ofpacts)->port = xport->ofp_port;
 
     return ofproto_dpif_execute_actions(xport->xbridge->ofproto, &flow, NULL,
-                                        &output.ofpact, sizeof output,
-                                        packet);
+                                        ofpacts.data, ofpacts.size, packet);
 }
 
 struct xlate_cache *
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 69f8518..7808a60 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -211,7 +211,7 @@  enum ofperr xlate_resume(struct ofproto_dpif *,
                          const struct ofputil_packet_in_private *,
                          struct ofpbuf *odp_actions, enum slow_path_reason *);
 
-int xlate_send_packet(const struct ofport_dpif *, struct dp_packet *);
+int xlate_send_packet(const struct ofport_dpif *, bool oam, struct dp_packet *);
 
 struct xlate_cache *xlate_cache_new(void);
 void xlate_push_stats(struct xlate_cache *, const struct dpif_flow_stats *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0cd1c25..d61add6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2219,7 +2219,7 @@  rstp_send_bpdu_cb(struct dp_packet *pkt, void *ofport_, void *ofproto_)
                      "does not have a configured source MAC address.",
                      ofproto->up.name, ofp_to_u16(ofport->up.ofp_port));
     } else {
-        ofproto_dpif_send_packet(ofport, pkt);
+        ofproto_dpif_send_packet(ofport, false, pkt);
     }
     dp_packet_delete(pkt);
 }
@@ -2243,7 +2243,7 @@  send_bpdu_cb(struct dp_packet *pkt, int port_num, void *ofproto_)
             VLOG_WARN_RL(&rl, "%s: cannot send BPDU on port %d "
                          "with unknown MAC", ofproto->up.name, port_num);
         } else {
-            ofproto_dpif_send_packet(ofport, pkt);
+            ofproto_dpif_send_packet(ofport, false, pkt);
         }
     }
     dp_packet_delete(pkt);
@@ -3093,7 +3093,7 @@  send_pdu_cb(void *port_, const void *pdu, size_t pdu_size)
                                  pdu_size);
         memcpy(packet_pdu, pdu, pdu_size);
 
-        ofproto_dpif_send_packet(port, &packet);
+        ofproto_dpif_send_packet(port, false, &packet);
         dp_packet_uninit(&packet);
     } else {
         VLOG_ERR_RL(&rl, "port %s: cannot obtain Ethernet address of iface "
@@ -3132,7 +3132,7 @@  bundle_send_learning_packets(struct ofbundle *bundle)
     LIST_FOR_EACH_POP (pkt_node, list_node, &packets) {
         int ret;
 
-        ret = ofproto_dpif_send_packet(pkt_node->port, pkt_node->pkt);
+        ret = ofproto_dpif_send_packet(pkt_node->port, false, pkt_node->pkt);
         dp_packet_delete(pkt_node->pkt);
         free(pkt_node);
         if (ret) {
@@ -4382,16 +4382,18 @@  group_dpif_get_selection_method(const struct group_dpif *group)
     return group->up.props.selection_method;
 }
 
-/* Sends 'packet' out 'ofport'.
+/* Sends 'packet' out 'ofport'. If 'port' is a tunnel and that tunnel type
+ * supports a notion of an OAM flag, sets it if 'oam' is true.
  * May modify 'packet'.
  * Returns 0 if successful, otherwise a positive errno value. */
 int
-ofproto_dpif_send_packet(const struct ofport_dpif *ofport, struct dp_packet *packet)
+ofproto_dpif_send_packet(const struct ofport_dpif *ofport, bool oam, 
+                         struct dp_packet *packet)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
     int error;
 
-    error = xlate_send_packet(ofport, packet);
+    error = xlate_send_packet(ofport, oam, packet);
 
     ovs_mutex_lock(&ofproto->stats_mutex);
     ofproto->stats.tx_packets++;
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 4034475..a3c2ac5 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -157,7 +157,8 @@  int ofproto_dpif_execute_actions__(struct ofproto_dpif *, const struct flow *,
 void ofproto_dpif_send_async_msg(struct ofproto_dpif *,
                                  struct ofproto_async_msg *);
 bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
-int ofproto_dpif_send_packet(const struct ofport_dpif *, struct dp_packet *);
+int ofproto_dpif_send_packet(const struct ofport_dpif *, bool oam,
+                             struct dp_packet *);
 void ofproto_dpif_flow_mod(struct ofproto_dpif *,
                            const struct ofputil_flow_mod *);
 struct rule_dpif *ofproto_dpif_refresh_rule(struct rule_dpif *);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index aad4904..c6a6af2 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2718,6 +2718,13 @@ 
           Set to an IPv4 address to set the IP address used as destination
           for transmitted BFD packets.  The default is <code>169.254.1.0</code>.
         </column>
+
+        <column name="bfd" key="oam">
+          Some tunnel protocols (such as Geneve) include a bit in the header
+          to indicate that the encapsulated packet is an OAM frame. By setting
+          this to true, BFD packets will be marked as OAM if encapsulated in
+          one of these tunnels.
+        </column>
       </group>
 
       <group title="BFD Status">