[ofa-general] Re: [PATCH] infiniband-diags/ibsysstat: use RMPP for client/server communication

[Sasha Khapyorsky]

Hi Hal,

On 11:10 Tue 27 Jan     , Hal Rosenstock wrote:
> 
> Just because the class allows RMPP doesn't mean all operations use it

Ok, agree. But obviously it doesn't mean that RMPP should not be used.
So I think we are fine now.

> so no this wasn't a bug IMO. It was done that way for extensibility
> (possibility of longer exchanges/using RMPP) which wouldn't have been
> possible with vendor range 1.

Ok, "bug" I used was too strong word (so I actually used bug/decision
:))

> >> What's the behavior of old client with new server and new client with
> >> old server ?
> >
> > Basically it works fine together. Old server responds short MAD with
> > RMPP flags inactive.
> 
> This is technically a protocol violation even though it may work.

Why? RMPP can be used or not, right? (If not why is flag active/inactive
bit needed?) Where do you see a violation?

Sasha

> > The only interesting case is when old client gets
> > long RMPP reply from new server - it will grab 256 bytes (so as it was
> > before the data will be truncated) and server will get timeout from RMPP
> > layer - and yes, seems we need to handle(drop) this MAD (as well as
> > possible RMPP unrelated timeout/error on client side). Something like
> > this:
> 
> Class version handling could also be used here to handle this.
> 
> -- Hal
> 
> > diff --git a/infiniband-diags/src/ibsysstat.c b/infiniband-diags/src/ibsysstat.c
> > index c20a6f0..a145daf 100644
> > --- a/infiniband-diags/src/ibsysstat.c
> > +++ b/infiniband-diags/src/ibsysstat.c
> > @@ -169,6 +174,11 @@ static char *ibsystat_serv(void)
> >        DEBUG("starting to serve...");
> >
> >        while ((umad = mad_receive(buf, -1))) {
> > +               if (umad_status(buf)) {
> > +                       DEBUG("drop mad with status %x: %s", umad_status(buf),
> > +                             strerror(umad_status(buf)));
> > +                       continue;
> > +               }
> >
> >                mad = umad_get_mad(umad);
> >
> > @@ -235,6 +245,9 @@ static char *ibsystat(ib_portid_t *portid, int attr)
> >        if (umad_recv(fd, buf, &len, timeout) < 0)
> >                IBPANIC("umad_recv failed.");
> >
> > +       if (umad_status(buf))
> > +               return strerror(umad_status(buf));
> > +
> >        DEBUG("Got sysstat pong..");
> >        if (attr != IB_PING_ATTR)
> >                puts(data);
> >
> >
> > Sasha
> >