[ovs-dev] [PATCH branch-1.6] ofproto: Don't allow feature reply to overflow max OpenFlow message size.

[Ben Pfaff]

Actually OF1.3 fixes this by moving port descriptions to a stats
reply.

On Wed, May 02, 2012 at 10:53:40AM -0700, Ben Pfaff wrote:
> Thanks, pushed to branch-1.6.
> 
> Unfortunately most of the information about ports in the feature reply
> doesn't appear in the port stats message, so just dropping the ports
> would lose information.
> 
> On Wed, May 02, 2012 at 10:48:51AM -0700, Justin Pettit wrote:
> > Looks good.
> > 
> > We should probably look at addressing this in the OpenFlow spec,
> > since my quick reading shows that the Features Reply has the same
> > behavior in OpenFlow 1.2.  I wonder if it would make sense to just
> > not report on ports there, since the controller can get that
> > information through a Port Stats message, which supports going
> > across multiple 64K messages.
> > 
> > --Justin
> > 
> > 
> > On May 2, 2012, at 10:35 AM, Ben Pfaff wrote:
> > 
> > > An OpenFlow message is limited to 65535 bytes so if there are more than
> > > (65535 - sizeof(struct ofp_switch_features)) / sizeof(struct ofp_phy_port)
> > > == 1364 ports then the feature reply cannot hold them all.  This commit
> > > prevents the feature reply from overflowing, at the cost of only reporting
> > > a random sampling of ports.
> > > 
> > > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > > ---
> > > This is already fixed on master because the refactoring for OpenFlow
> > > 1.[123] support was carefully done to avoid overflows.
> > > 
> > > This isn't really an issue before branch-1.6 because only branch-1.6
> > > and master support more than 1024 ports in the kernel datapath.  I
> > > guess some third-party hardware datapath could support more but I am
> > > not aware of one.
> > > 
> > > ofproto/ofproto.c |    3 +++
> > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > > index 28bed08..86cf0bc 100644
> > > --- a/ofproto/ofproto.c
> > > +++ b/ofproto/ofproto.c
> > > @@ -1904,6 +1904,9 @@ handle_features_request(struct ofconn *ofconn, const struct ofp_header *oh)
> > >     osf->actions = htonl(actions);
> > > 
> > >     HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
> > > +        if (buf->size + sizeof port->opp > UINT16_MAX) {
> > > +            break;
> > > +        }
> > >         ofpbuf_put(buf, &port->opp, sizeof port->opp);
> > >     }
> > > 
> > > -- 
> > > 1.7.2.5
> > > 
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > http://openvswitch.org/mailman/listinfo/dev
> >