Attention is currently required from: fixeria.

neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email )

Change subject: add VTY 'gtp-echo' command ......................................................................

Patch Set 1:

(3 comments)

File src/osmo-upf/upf_vty.c:

https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/d3d76587_487cbd22?usp=... : PS1, Line 491: tx to

The command vector looks overloaded to me: […]

Would you accept this?

gtp-echo tx remote-ip IP_ADDR [(local-ip|local-dev)] [IP_ADDR_OR_DEV]

The reason i sprinkle these words in between is to allow future extensions to the vty language.

When arguments have no explicit prefix like 'gtp-echo REMOTE_ADDR', then future 'gtp-echo' commands are prone to have conflicts. What if we want to allow resolving a hostname (stupid example but..). What then if a user wants to use a hostname that is the same as some other vty token that an alternative command offers in the same place.

I've faced such situations before, so I would much prefer to explicitly name each argument.

I agree that 'tx to' doesn't seem nice, but think semantically, inserting 'foo' for arbitrary future extensions:

``` gtp-echo |- gtp-echo tx | |- to | |- from | | |- ip | | |- dev | |- foo |- gtp-echo foo ```

That is my idea behind it: the "tx" is one branch, the "to" a sub-branch of the argument logic. I thought somewhat about this but would like to keep those keywords in there. Is that too future paranoid?

https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/74265275_b829a746?usp=... : PS1, Line 556: %%Error

cosmetic: missing space

missing space?

File tests/gtp-echo.vty:

https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/e9563e63_dde20d69?usp=... : PS1, Line 2: show pid : show uptime : show version : show online-help : list [with-flags] : exit : help : show vty-attributes : show vty-attributes (application|library|global) : enable [expert-mode] : terminal length <0-512> : terminal no length :

these lines (and more below), for obvious reasons, shall not be part of the VTY test

oof, got so annoyed from removing them every time that i just left them in while testing ... and forgot! thanks