netdev - Re: [PATCH iproute2-next] f_flower: implement pfcp opts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230615084603.20d04815@hermes.local>
Date: Thu, 15 Jun 2023 08:46:03 -0700
From: Stephen Hemminger <stephen@...workplumber.org>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: netdev@...r.kernel.org, dsahern@...il.com, Michal Swiatkowski
 <michal.swiatkowski@...ux.intel.com>
Subject: Re: [PATCH iproute2-next] f_flower: implement pfcp opts

On Thu, 15 Jun 2023 10:36:51 +0200
Marcin Szycik <marcin.szycik@...ux.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@...ux.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.