From: Stephen Hemminger <stephen@networkplumber.org>
To: Marcin Szycik <marcin.szycik@linux.intel.com>
Cc: netdev@vger.kernel.org, dsahern@gmail.com,
Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Subject: Re: [PATCH iproute2-next] f_flower: implement pfcp opts
Date: Thu, 15 Jun 2023 08:46:03 -0700 [thread overview]
Message-ID: <20230615084603.20d04815@hermes.local> (raw)
In-Reply-To: <a22c8701-0663-2c27-d866-492e7655fe5e@linux.intel.com>
On Thu, 15 Jun 2023 10:36:51 +0200
Marcin Szycik <marcin.szycik@linux.intel.com> wrote:
> On 14.06.2023 18:54, Stephen Hemminger wrote:
> > On Wed, 14 Jun 2023 11:17:58 +0200
> > Marcin Szycik <marcin.szycik@linux.intel.com> wrote:
> >
> >> +static void flower_print_pfcp_opts(const char *name, struct rtattr *attr,
> >> + char *strbuf, int len)
> >> +{
> >> + struct rtattr *tb[TCA_FLOWER_KEY_ENC_OPT_PFCP_MAX + 1];
> >> + struct rtattr *i = RTA_DATA(attr);
> >> + int rem = RTA_PAYLOAD(attr);
> >> + __be64 seid;
> >> + __u8 type;
> >> +
> >> + parse_rtattr(tb, TCA_FLOWER_KEY_ENC_OPT_PFCP_MAX, i, rem);
> >> + type = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_PFCP_TYPE]);
> >> + seid = rta_getattr_be64(tb[TCA_FLOWER_KEY_ENC_OPT_PFCP_SEID]);
> >> +
> >> + snprintf(strbuf, len, "%02x:%llx", type, seid);
> >> +}
> >> +
> >
> > NAK you need to support JSON output.
> > Also whet if kernel gives partial data.
>
> Hmm... when we were adding GTP opts parsing, you requested to remove JSON
> support [1]. Are you sure you want us to add it here?
>
> Could you elaborate about partial data? Are there any similar functions
> which properly handle partial data?
>
> [1] https://lore.kernel.org/netdev/20220127091541.6667d4d1@hermes.local
The handling of JSON in flower needs lots of cleanup work, not related to this patch.
So changing my mind. This patch is OK, it just continues existing unique model of
handling.
Ideally, there would not be all the special case handling of JSON in flower.
And the code would be more robust about getting partial attributes back from
the kernel. Other parts of tc are not as brittle.
prev parent reply other threads:[~2023-06-15 15:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-14 9:17 [PATCH iproute2-next] f_flower: implement pfcp opts Marcin Szycik
2023-06-14 16:54 ` Stephen Hemminger
2023-06-15 8:36 ` Marcin Szycik
2023-06-15 15:46 ` Stephen Hemminger [this message]
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=20230615084603.20d04815@hermes.local \
--to=stephen@networkplumber.org \
--cc=dsahern@gmail.com \
--cc=marcin.szycik@linux.intel.com \
--cc=michal.swiatkowski@linux.intel.com \
--cc=netdev@vger.kernel.org \
/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).