Re: [dpdk-dev] [PATCH v3 3/5] examples/l3fwd: add FIB infrastructure

["Walsh, Conor" <conor.walsh@intel.com>]

<snip>
> >   #define CMD_LINE_OPT_CONFIG "config"
> > @@ -596,6 +611,10 @@ parse_args(int argc, char **argv)
> >   			l3fwd_lpm_on = 1;
> >   			break;
> >
> > +		case 'F':
> > +			l3fwd_fib_on = 1;
> > +			break;
> > +
> >   		/* long options */
> 

Hi Anatoly,
Thanks for your feedback responses inline below.

> IMO this necessitates creating an enum instead of a set of mutually
> exclusive binary switches. For example:
> 
> enum L3FWD_LOOKUP_MODE {
>      L3FWD_LOOKUP_DEFAULT,
>      L3FWD_LOOKUP_LPM,
>      L3FWD_LOOKUP_EM,
>      L3FWD_LOOKUP_FIB
> };
> static enum L3FWD_LOOKUP_MODE lookup_mode;

I agree I have added this to the v4 of the patchset.

> 
> ...
> 
> case 'F':
>      if (lookup_mode != L3FWD_LOOKUP_DEFAULT) {
>          RTE_LOG(ERR, L3FWD, "Only one lookup mode is allowed at a time!");
>          return -1;
>      }
>      lookup_mode = L3FWD_LOOKUP_FIB;
>      break;
> ...
> 
> Additionally, i really think this '-E'/'-L'/'-F' business is silly and
> it would be better to have something like "--lookup=exact|lpm|fib", and
> leave the old -E/-L options for backwards compatibility.

I have also added the above to v4 of the patchset, which will be released soon.
These changes should make it easier to use for users if more lookup methods are added to l3fwd.

Thanks,
Conor.

> 
> --
> Thanks,
> Anatoly