On Thursday 23 September 2010, John Fastabend wrote:
> On 8/25/2010 5:27 AM, Jens Osterkamp wrote:
> > This patch contains the first part of an initial implementation of the
> > IEEE 802.1Qbg standard: it implements code for the exchange of EVB
> > capabilities between a host with virtual machines and an adjacent switch.
> > For this it adds a new EVB TLV to LLDP.
> >
> > Exchange of EVB TLV may be enabled or disabled on a per port basis.
> > Information about the information negotiated by the protocol can be
> > queried on the commandline with lldptool.
> >
> > This patch adds support for querying and setting parameters used in
> > the exchange of EVB TLV messages.
> > The parameters that can be set are:
> >
> > - forwarding mode
> > - host protocol capabilities (RTE, ECP, VDP)
> > - no. of supported VSIs
> > - retransmission timer exponent (RTE)
> >
> > The parameters are implemented as a local policy: all frames received by
> > an adjacent switch are validated against this policy and taken over where
> > appropriate. Negotiated parameters are stored in lldpads config, picked up
> > again and used at the next start.
> >
> > The patch applies to lldpad 0.9.38 and still contains code to log protocol
> > activity more verbosely than it would be necessary in the final version.
> >
> > Signed-off-by: Jens Osterkamp <jens@xxxxxxxxxxxxxxxxxx>
snip
> remove the unneeded braces in the if statements above.
>
> > + /* only look at the mode for now
> > + if (tie->ccap == ed->tie->ccap) {
> > + ed->tie->ccap = tie->ccap;
> > + }
> > +
> > + if (tie->cvsi == ed->tie->cvsi) {
> > + ed->tie->cvsi = tie->cvsi;
> > + }
> > + */
>
> Why is this commented out? I will review the spec shortly, but please
> remind me. If its not needed remove it and add it later in a clean
> patch
Its not used right now, so its commented out. I removed it for now.
>
> > +
> > + return TLV_OK;
> > +}
> > +
> > +/* evb_compare
> > + *
> > + * compare our own and received tlv_info_evb
> > + */
> > +static int evb_compare(struct evb_data *ed, struct tlv_info_evb *tie)
> > +{
> > + printf("%s(%i): \n", __func__, __LINE__);
> > +
> > + if ((ed->tie->cmode == tie->cmode) /* &&
> > + (ed->tie->ccap == tie->ccap) &&
> > + (ed->tie->cvsi == tie->cvsi)*/)
>
> Same here if its going to be commented out lets remove it for now.
Removed it.
>
> > + return 0;
> > + else
> > + return 1;
> > +}
> > +
> > +/* evb_statemachine:
> > + *
> > + * handle possible states during EVB capabilities exchange
> > + *
> > + * possible states: EVB_OFFER_CAPABILITIES
> > + * EVB_CONFIGURE
> > + * EVB_CONFIRMATION
> > + */
> > +static void evb_statemachine(struct evb_data *ed, struct tlv_info_evb *tie)
snip
> > diff --git a/lldptool.c b/lldptool.c
> > index 5cf2846..7e166fe 100644
> > --- a/lldptool.c
> > +++ b/lldptool.c
> > @@ -39,6 +39,7 @@
> > #include "lldp_med_clif.h"
> > #include "lldp_8023_clif.h"
> > #include "lldp_dcbx_clif.h"
> > +#include "lldp_evb_clif.h"
> > #include "lldptool.h"
> > #include "lldptool_cli.h"
> > #include "lldp_mod.h"
> > @@ -156,6 +157,7 @@ struct lldp_module *(*register_tlv_table[])(void) = {
> > ieee8023_cli_register,
> > med_cli_register,
> > dcbx_cli_register,
> > + evb_cli_register,
> > NULL,
> > };
> >
>
> Jens, in general this looks OK. Can you fix the coding style throughout
> I prefer for single line if statements,
>
> if (x)
> a_line;
> else (y)
> b_line;
>
> The intent here is to use the same coding style as linux kernel where it
> is reasonable to do so. Like I said before I know lldpad is not at all
> consistent in this regard, but I want to get the rest of the code in
> order and certainly have new code be consistent.
fixed this in all the places I could find.
I will address the rest your comments in my next posting of the series.
Thanks !
Jens
--
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization
