diff mbox

[ovs-dev] stp: Set time of BPDU packet.

Message ID 1488973990-62421-1-git-send-email-nic@opencloud.tech
State Rejected
Headers show

Commit Message

nickcooper-zhangtonghao March 8, 2017, 11:53 a.m. UTC 
The OvS BPDU packets have not right format. The STP works
well in OvS bridges when stp enabled, because they set the
time(e.g. max age, hello time and forward delay) in ticks.
But they should be sec. If so,  OvS STP can work well with
other type of bridge using STP.

Signed-off-by: nickcooper-zhangtonghao <nic@opencloud.tech>
---
 lib/stp.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Ben Pfaff March 17, 2017, 10:56 p.m. UTC | #1 
On Wed, Mar 08, 2017 at 03:53:10AM -0800, nickcooper-zhangtonghao wrote:
> The OvS BPDU packets have not right format. The STP works
> well in OvS bridges when stp enabled, because they set the
> time(e.g. max age, hello time and forward delay) in ticks.
> But they should be sec. If so,  OvS STP can work well with
> other type of bridge using STP.
> 
> Signed-off-by: nickcooper-zhangtonghao <nic@opencloud.tech>

Timer values in spanning tree BPDUs are in 1/256 of a second, not in
seconds.  Are you sure there's a bug here?
nickcooper-zhangtonghao March 18, 2017, 8:11 a.m. UTC | #2 
I was wrong.

Thanks.
Nick

> On Mar 18, 2017, at 6:56 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Wed, Mar 08, 2017 at 03:53:10AM -0800, nickcooper-zhangtonghao wrote:
>> The OvS BPDU packets have not right format. The STP works
>> well in OvS bridges when stp enabled, because they set the
>> time(e.g. max age, hello time and forward delay) in ticks.
>> But they should be sec. If so,  OvS STP can work well with
>> other type of bridge using STP.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <nic@opencloud.tech <mailto:nic@opencloud.tech>>
> 
> Timer values in spanning tree BPDUs are in 1/256 of a second, not in
> seconds.  Are you sure there's a bug here?
diff mbox

Patch

diff --git a/lib/stp.c b/lib/stp.c
index d828c64..b9eac1c 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -1065,13 +1065,14 @@  stp_transmit_config(struct stp_port *p) OVS_REQUIRES(mutex)
         if (root) {
             config.message_age = htons(0);
         } else {
-            config.message_age = htons(stp->root_port->message_age_timer.value
-                                       + MESSAGE_AGE_INCREMENT);
+            config.message_age =
+                htons(timer_to_ms(stp->root_port->message_age_timer.value) / 1000
+                      + MESSAGE_AGE_INCREMENT);
         }
-        config.max_age = htons(stp->max_age);
-        config.hello_time = htons(stp->hello_time);
-        config.forward_delay = htons(stp->forward_delay);
-        if (ntohs(config.message_age) < stp->max_age) {
+        config.max_age = htons(timer_to_ms(stp->max_age) / 1000);
+        config.hello_time = htons(timer_to_ms(stp->hello_time) / 1000);
+        config.forward_delay = htons(timer_to_ms(stp->forward_delay) / 1000);
+        if (ntohs(config.message_age) < timer_to_ms(stp->max_age) / 1000) {
             p->topology_change_ack = false;
             p->config_pending = false;
             VLOG_DBG_RL(&stp_rl, "bridge: %s, port: %s, transmit config bpdu",
@@ -1108,7 +1109,8 @@  stp_record_config_information(struct stp_port *p,
     p->designated_cost = ntohl(config->root_path_cost);
     p->designated_bridge = ntohll(config->bridge_id);
     p->designated_port = ntohs(config->port_id);
-    stp_start_timer(&p->message_age_timer, ntohs(config->message_age));
+    stp_start_timer(&p->message_age_timer,
+                    ms_to_timer(ntohs(config->message_age) * 1000));
 }
 
 static void
@@ -1116,9 +1118,9 @@  stp_record_config_timeout_values(struct stp *stp,
                                  const struct stp_config_bpdu  *config)
      OVS_REQUIRES(mutex)
 {
-    stp->max_age = ntohs(config->max_age);
-    stp->hello_time = ntohs(config->hello_time);
-    stp->forward_delay = ntohs(config->forward_delay);
+    stp->max_age = ms_to_timer(ntohs(config->max_age) * 1000);
+    stp->hello_time = ms_to_timer(ntohs(config->hello_time) * 1000);
+    stp->forward_delay = ms_to_timer(ntohs(config->forward_delay) * 1000);
     stp->topology_change = config->flags & STP_CONFIG_TOPOLOGY_CHANGE;
 }