[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170206181515.GA19171@salvia>
Date: Mon, 6 Feb 2017 19:15:15 +0100
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Jonas Bonn <jonas@...thpole.se>, netdev@...r.kernel.org
Cc: Harald Welte <laforge@...monks.org>
Subject: Re: [PATCH 1/1] gtp: support SGSN-side tunnels
On Mon, Feb 06, 2017 at 03:16:22PM +0100, Harald Welte wrote:
> Hi Jonas,
>
> On Mon, Feb 06, 2017 at 02:33:07PM +0100, Jonas Bonn wrote:
> > Fair enough. The use-case I am looking at involves PGW load-testing where
> > the simulated load is generated locally on the SGSN so it _is_ seeing IP
> > packets and the SNDCP is left out altogether.
>
> Ok, it would have been useful to document that test-only feature in the
> changelog and/or code. Like "support simulated RAN-side tunnels" or
> "support SGSN/S-GW simulation".
Right. Please, include this in your follow up v2 patch description.
BTW, please also indicate [PATCH net-next] for new features.
> > Perhaps this is too pathological to warrant messing with the upstream
> > driver... I don't know: the symmetry does not cost much even if it's
> > of limited use.
>
> There are plenty of features in the mainline kernel related to testing,
> see pktgen for example. So I think if it doesn't impose complexity,
> performance issues or stretches the existing architecture, I think
> there's no reason to keep it out.
>
> Looking at the code, I think the one conditional on the flags is not
> going to kill significant performance of the "normal" use case. But
> that's of course just guessing, without any benchmark to back that up.
>
> Semantically, I'm not sure if the FLAGS and the re-use of the
> SGSN_ADDRESS TLV is the best choice. If suddenly the meaning of the TLV
> is "Peer GSN Address" then it should be called that way. We could have
> a #define SGSN_ADDRESS to GSN_PEER_ADDRESS to make old code compile.
> I'll let Pablo respond to this as he came up with the netlink interface,
> as far as I can remember :)
I agree with Harald that a new netlink TLV, ie. IFLA_GTP_MODE, to
indicate if this is expecting to operate on the GGSN or SGSN side
would be better. See include/uapi/linux/if_link.h.
Flags allows us to combine different features, in this case we won't
combine anything since these two modes are mutually exclusive.
> Also, like with any changes to the kernel and netlink interface code, I
> think we should always mandate similar changes to be made to libgtpnl so
> the feature can actually be used/tested with the standard
> tools/utilities available to anyone.
Yes, at least some scripts and short text file example would suffice.
Thanks!
Powered by blists - more mailing lists