On Sat, Feb 22, 2014 at 12:42 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Fri, Feb 21, 2014 at 10:07:14AM +0800, Kmindg G wrote:
>> On Fri, Feb 21, 2014 at 5:19 AM, Ben Pfaff <b...@nicira.com> wrote:
>> > On Thu, Feb 20, 2014 at 12:45:49PM +0800, Kmindg G wrote:
>> >> On Thu, Feb 20, 2014 at 3:20 AM, Ben Pfaff <b...@nicira.com> wrote:
>> >> > Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has
>> >> > waited
>> >> > for ports to notify it that their status has changed before it sends a
>> >> > port status update to controllers.
>> >> >
>> >> > Also, Open vSwitch never sent port config updates at all for port
>> >> > modifications other than OFPPC_PORT_DOWN. I guess that this is a relic
>> >> > from the era when there was only one controller, since presumably the
>> >> > controller already knew that it changed the port configuration. But in
>> >> > the
>> >> > multi-controller world, it makes sense to send such port status updates,
>> >> > and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
>> >> > shouldn't be sent.
>> >> >
>> >> > Signed-off-by: Ben Pfaff <b...@nicira.com>
>> >> > Reported-by: Kmindg G <kmi...@gmail.com>
>> >> > ---
>> >> > AUTHORS | 1 +
>> >> > ofproto/ofproto.c | 25 +++++++++++++------------
>> >> > 2 files changed, 14 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/AUTHORS b/AUTHORS
>> >> > index c557303..2fda8d7 100644
>> >> > --- a/AUTHORS
>> >> > +++ b/AUTHORS
>> >> > @@ -197,6 +197,7 @@ John Hurley john.hur...@netronome.com
>> >> > Kevin Mancuso kevin.manc...@rackspace.com
>> >> > Kiran Shanbhog ki...@vmware.com
>> >> > Kirill Kabardin
>> >> > +Kmindg G kmi...@gmail.com
>> >> > Koichi Yagishita yagishita.koi...@jrc.co.jp
>> >> > Konstantin Khorenko khore...@openvz.org
>> >> > Kris zhang zhang.k...@gmail.com
>> >> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> >> > index 62c97a2..48e10ca 100644
>> >> > --- a/ofproto/ofproto.c
>> >> > +++ b/ofproto/ofproto.c
>> >> > @@ -2986,22 +2986,23 @@ update_port_config(struct ofport *port,
>> >> > enum ofputil_port_config config,
>> >> > enum ofputil_port_config mask)
>> >> > {
>> >> > - enum ofputil_port_config old_config = port->pp.config;
>> >> > - enum ofputil_port_config toggle;
>> >> > -
>> >> > - toggle = (config ^ port->pp.config) & mask;
>> >> > - if (toggle & OFPUTIL_PC_PORT_DOWN) {
>> >> > - if (config & OFPUTIL_PC_PORT_DOWN) {
>> >> > - netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL);
>> >> > - } else {
>> >> > - netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL);
>> >> > - }
>> >> > + enum ofputil_port_config toggle = (config ^ port->pp.config) &
>> >> > mask;
>> >> > +
>> >> > + if (toggle & OFPUTIL_PC_PORT_DOWN
>> >> > + && (config & OFPUTIL_PC_PORT_DOWN
>> >> > + ? netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL)
>> >> > + : netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL))) {
>> >> > + /* We tried to bring the port up or down, but it failed, so
>> >> > don't
>> >> > + * update the "down" bit. */
>> >> > toggle &= ~OFPUTIL_PC_PORT_DOWN;
>> >> > }
>> >> >
>> >> > - port->pp.config ^= toggle;
>> >> > - if (port->pp.config != old_config) {
>> >> > + if (toggle) {
>> >> > + enum ofputil_port_config old_config = port->pp.config;
>> >> > + port->pp.config ^= toggle;
>> >> > port->ofproto->ofproto_class->port_reconfigured(port,
>> >> > old_config);
>> >> > + connmgr_send_port_status(port->ofproto->connmgr, &port->pp,
>> >> > + OFPPR_MODIFY);
>> >> > }
>> >> > }
>> >> >
>> >> > --
>> >> > 1.7.10.4
>> >> >
>> >>
>> >> looks good to me.
>> >
>> > Thanks. What do you think of this version? I think that it retains
>> > better compatibility with OpenFlow 1.0 in particular.
>> >
>> > --8<--------------------------cut here-------------------------->8--
>> >
>> > From: Ben Pfaff <b...@nicira.com>
>> > Date: Thu, 20 Feb 2014 13:18:51 -0800
>> > Subject: [PATCH] ofproto: Send port status message for port-mods, right
>> > away.
>> >
>> > Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has waited
>> > for ports to notify it that their status has changed before it sends a
>> > port status update to controllers.
>> >
>> > Also, Open vSwitch never sent port config updates at all for port
>> > modifications other than OFPPC_PORT_DOWN. I guess that this is a relic
>> > from the era when there was only one controller, since presumably the
>> > controller already knew that it changed the port configuration. But in the
>> > multi-controller world, it makes sense to send such port status updates,
>> > and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
>> > shouldn't be sent.
>> >
>> > Signed-off-by: Ben Pfaff <b...@nicira.com>
>> > Reported-by: Kmindg G <kmi...@gmail.com>
>> > ---
>> > ofproto/connmgr.c | 30 ++++++++++++++++++++++++++++--
>> > ofproto/connmgr.h | 4 ++--
>> > ofproto/ofproto.c | 38 ++++++++++++++++++++------------------
>> > 3 files changed, 50 insertions(+), 22 deletions(-)
>> >
>> > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
>> > index a58e785..7a25828 100644
>> > --- a/ofproto/connmgr.c
>> > +++ b/ofproto/connmgr.c
>> > @@ -1456,9 +1456,11 @@ static void schedule_packet_in(struct ofconn *,
>> > struct ofproto_packet_in,
>> > enum ofp_packet_in_reason wire_reason);
>> >
>> > /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to
>> > appropriate
>> > - * controllers managed by 'mgr'. */
>> > + * controllers managed by 'mgr'. For messages caused by a controller
>> > + * OFPT_PORT_MOD, specify 'source' as the controller connection that sent
>> > the
>> > + * request; otherwise, specify 'source' as NULL. */
>> > void
>> > -connmgr_send_port_status(struct connmgr *mgr,
>> > +connmgr_send_port_status(struct connmgr *mgr, struct ofconn *source,
>> > const struct ofputil_phy_port *pp, uint8_t
>> > reason)
>> > {
>> > /* XXX Should limit the number of queued port status change messages.
>> > */
>> > @@ -1471,6 +1473,30 @@ connmgr_send_port_status(struct connmgr *mgr,
>> > if (ofconn_receives_async_msg(ofconn, OAM_PORT_STATUS, reason)) {
>> > struct ofpbuf *msg;
>> >
>> > + /* Before 1.3.4, OpenFlow specified that OFPT_PORT_MOD should
>> > not
>> > + * generate OFPT_PORT_STATUS messages. That requirement was a
>> > + * relic of how OpenFlow originally supported a single
>> > controller,
>> > + * so that one could expect the controller to already know the
>> > + * changes it had made.
>> > + *
>> > + * OpenFlow 1.3.4 and OpenFlow 1.5 changed OFPT_PORT_MOD to
>> > send
>> > + * OFPT_PORT_STATUS messages to every controller. This is
>> > + * obviously more useful in the multi-controller case. We
>> > could
>> > + * always implement it that way in OVS, but that would risk
>> > + * confusing controllers that are intended for
>> > single-controller
>> > + * use only. (Imagine a controller that generates an
>> > + * OFPT_PORT_MOD in response to any OFPT_PORT_STATUS!)
>> > + *
>> > + * So this compromises: for OpenFlow 1.0, 1.1, and 1.2, it
>> > + * generates OFPT_PORT_STATUS for OFPT_PORT_MOD, but not back
>> > to
>> > + * the originating controller. In a single-controller
>> > environment,
>> > + * in particular, this means that it will never generate
>> > + * OFPT_PORT_STATUS for OFPT_PORT_MOD at all. */
>> > + if (ofconn == source
>> > + && rconn_get_version(ofconn->rconn) < OFP13_VERSION) {
>> > + continue;
>> > + }
>> > +
>>
>>
>> I'm not very familiar with the openflow specification, but I find that
>> it says: "If the port config bits are changed by the switch through
>> another administrative interface, the switch sends an OFPT_PORT_STATUS
>> message to notify the controller of the change."in openflow-spec-1.1.0
>> A.2.1. Did I misunderstand something?
>
> "another administrative interface" means "other than OpenFlow".
All right, this patch is good to me.
Thanks.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
