Re: [dpdk-dev] [EXT] RE: [PATCH v2 09/12] examples/ipsec-secgw: add eventmode to ipsec-secgw
From: Ananyev, Konstantin <hidden>
Date: 2020-01-30 11:13:39
Hi Lukasz,
quoted
quoted
/* * 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.quoted
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.quoted
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