Hi Lukasz,

> >>
> >>  /*
> >>   * RX/TX HW offload capabilities to enable/use on ethernet ports.
> >> @@ -1185,7 +1205,7 @@ main_loop(__attribute__((unused)) void *dummy)
> >>  }
> >>
> >>  static int32_t
> >> -check_params(void)
> >> +check_params(struct eh_conf *eh_conf)
> >>  {
> >>    uint8_t lcore;
> >>    uint16_t portid;
> >> @@ -1220,6 +1240,14 @@ check_params(void)
> >>                    return -1;
> >>            }
> >>    }
> >> +
> >> +  if (eh_conf->mode == EH_PKT_TRANSFER_MODE_POLL) {
> >> +          if (schedule_type) {
> >> +                  printf("error: option --schedule-type applies only to 
> >> event mode\n");
> >> +                  return -1;
> >> +          }
> >> +  }
> >
> > As a nit - might be better to keep check_params() intact,
> > and put this new check above into a separate function?
> > check_eh_conf() or so?
> 
> [Lukasz] I will put the check into new check_eh_conf() function.
> 
> > Another thing it seems a bit clumsy construction to have global var 
> > (scheduler_type)
> > just to figure out was particular option present on command line or not.
> > Probably simler way to avoid it - set initially 
> > em_conf->ext_params.sched_type to
> > some invalid value (-1 or so). Then after parse args you can check did its 
> > value
> > change or not.
> 
> [Lukasz] I will change it in V3.
> 
> > As alternative thought: wouldn't it be better to unite both  --transfer-mode
> > and --schedule-type options into one?
> > Then possible values for this unite option would be:
> > "poll"
> > "event" (expands to "event-ordered")
> > "event-ordered"
> > "event-atomic"
> > "event-parallel"
> > And this situation you are checking above simply wouldn't be possible.
> > Again probably would be easier/simpler for users.
> 
> [Lukasz] I would rather not combine event mode parameters into one for two 
> reason:
> - to be consistent with poll where one configuration item is controlled with 
> one option,
> - if we come up with a need to add a new event mode parameter in future then 
> we
> we will need to split event-ordered back to --transfer-mode and 
> --schedule-type
> to be consistent with how with provide event mode command line options.

I thought for future mods we can just keep adding new types here:
"event-xxx", "poll-yyy", etc.
But if you think separate ones is a better approach - I am fine. 
Konstantin

Reply via email to