From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Nicolau, Radu" <radu.nicolau@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: fix portmask option parsing
Date: Thu, 21 Jun 2018 15:02:02 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258C0C4009C@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <36d2a552-e043-7451-ed66-70d13f0f6fe6@nxp.com>
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, June 21, 2018 2:49 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: Nicolau, Radu <radu.nicolau@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: fix portmask option parsing
>
> Hi Konstantin,
>
> On 6/5/2018 7:46 PM, Konstantin Ananyev wrote:
> > parse_portmask() returns both portmask value and possible error code
> > as 32-bit integer. That causes some confusion for callers.
> > Split error code and portmask value into two distinct variables.
> > Also allows to run the app with unprotected_port_mask == 0.
>
> This would also allow cryptodev_mask == 0 to work well which should not be the case.
>
> >
> > Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > examples/ipsec-secgw/ipsec-secgw.c | 29 +++++++++++++++--------------
> > 1 file changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> > index fafb41161..5d7071657 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -972,20 +972,19 @@ print_usage(const char *prgname)
> > }
> >
> > static int32_t
> > -parse_portmask(const char *portmask)
> > +parse_portmask(const char *portmask, uint32_t *pmv)
> > {
> > - char *end = NULL;
> > + char *end;
> > unsigned long pm;
> >
> > /* parse hexadecimal string */
> > + errno = 0;
> > pm = strtoul(portmask, &end, 16);
> > - if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
> > + if (errno != 0 || *end != '\0' || pm > UINT32_MAX)
> > return -1;
> >
> > - if ((pm == 0) && errno)
> > - return -1;
> > -
> > - return pm;
> > + *pmv = pm;
> > + return 0;
> > }
> >
> > static int32_t
> > @@ -1063,6 +1062,7 @@ parse_args(int32_t argc, char **argv)
> > int32_t opt, ret;
> > char **argvopt;
> > int32_t option_index;
> > + uint32_t v;
> > char *prgname = argv[0];
> > int32_t f_present = 0;
> >
> > @@ -1073,8 +1073,8 @@ parse_args(int32_t argc, char **argv)
> >
> > switch (opt) {
> > case 'p':
> > - enabled_port_mask = parse_portmask(optarg);
> > - if (enabled_port_mask == 0) {
> > + ret = parse_portmask(optarg, &enabled_port_mask);
> > + if (ret < 0 || enabled_port_mask == 0) {
> > printf("invalid portmask\n");
> > print_usage(prgname);
> > return -1;
> > @@ -1085,8 +1085,8 @@ parse_args(int32_t argc, char **argv)
> > promiscuous_on = 1;
> > break;
> > case 'u':
> > - unprotected_port_mask = parse_portmask(optarg);
> > - if (unprotected_port_mask == 0) {
> > + ret = parse_portmask(optarg, &unprotected_port_mask);
> > + if (ret < 0) {
> > printf("invalid unprotected portmask\n");
> > print_usage(prgname);
> > return -1;
> > @@ -1147,15 +1147,16 @@ parse_args(int32_t argc, char **argv)
> > single_sa_idx);
> > break;
> > case CMD_LINE_OPT_CRYPTODEV_MASK_NUM:
> > - ret = parse_portmask(optarg);
> > + ret = parse_portmask(optarg, &v);
>
> I think there is no need for v, enabled_cryptodev_mask can be used instead.
Right now - it can't as enabled_cryptodevmask is uint64_t.
To do what you suggesting we have either downgrade enabled_cryptodevmask 32-bits,
or upgrade enabled_port_mask to 64-bit and change parse_portmask() to accept 64-bit parameter.
>
> > if (ret == -1) {
>
> enabled_cryptodev_mask should not be 0 and should be checked here.
Could you explain a bit more why enabled_cryptodevmask==0 is not allowed?
Konstantin
next prev parent reply other threads:[~2018-06-21 15:02 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-05 14:16 [dpdk-dev] [PATCH 1/2] examples/ipsec-secgw: fix bypass rule processing for outbound port Konstantin Ananyev
2018-06-05 14:16 ` [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: fix portmask option parsing Konstantin Ananyev
2018-06-05 15:36 ` Iremonger, Bernard
2018-06-21 13:48 ` Akhil Goyal
2018-06-21 15:02 ` Ananyev, Konstantin [this message]
2018-06-22 10:00 ` Akhil Goyal
2018-06-22 10:10 ` Ananyev, Konstantin
2018-06-22 10:40 ` Akhil Goyal
2018-06-22 11:51 ` Ananyev, Konstantin
2018-07-05 9:03 ` Akhil Goyal
2018-07-24 8:48 ` De Lara Guarch, Pablo
2018-07-24 12:37 ` Ananyev, Konstantin
2018-07-24 12:49 ` Akhil Goyal
2018-07-24 13:04 ` Ananyev, Konstantin
2018-06-21 13:25 ` [dpdk-dev] [PATCH 1/2] examples/ipsec-secgw: fix bypass rule processing for outbound port Akhil Goyal
2018-07-24 16:30 ` De Lara Guarch, Pablo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB977258C0C4009C@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=akhil.goyal@nxp.com \
--cc=dev@dpdk.org \
--cc=radu.nicolau@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).