Re: [PATCH 1/1] gtp: support SGSN-side tunnels

[Pablo Neira Ayuso <pablo@netfilter.org>]

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!