All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] TST_GET_UNUSED_PORT returns ports < 1024
@ 2019-05-29  9:33 Christian Amann
  2019-05-30 19:00 ` Petr Vorel
  2019-06-06 13:43 ` Cyril Hrubis
  0 siblings, 2 replies; 5+ messages in thread
From: Christian Amann @ 2019-05-29  9:33 UTC (permalink / raw)
  To: ltp

Hi,

when using the TST_GET_UNUSED_PORT macro you sometimes get ports lower
than 1024 which would require a testcase to have the
CAP_NET_BIND_SERVICE capability (or simply run as root).

Of course you could write a wrapper like the following to avoid that
issue, but in my opinion it would be nice to have the option to get
non-root ports directly from the library.

/* Wrapper to to get a non-root port if necessary */
static int get_port(uid_t uid)
{
        static int count = 10;
        int port;

        port = TST_GET_UNUSED_PORT(AF_INET, SOCK_STREAM);
        if (port < 1000 && uid != 0) {
                if (!count)
                        tst_brk(TBROK, "Could not get fitting port");
                count--;
                return get_port(uid);
        }
        return port;
}

Kind regards,

Christian


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190529/18f11994/attachment.html>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] TST_GET_UNUSED_PORT returns ports < 1024
  2019-05-29  9:33 [LTP] TST_GET_UNUSED_PORT returns ports < 1024 Christian Amann
@ 2019-05-30 19:00 ` Petr Vorel
  2019-06-06 13:43 ` Cyril Hrubis
  1 sibling, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2019-05-30 19:00 UTC (permalink / raw)
  To: ltp

Hi Christian,

> Hi,

> when using the TST_GET_UNUSED_PORT macro you sometimes get ports lower
> than 1024 which would require a testcase to have the
> CAP_NET_BIND_SERVICE capability (or simply run as root).
That surprised me. I thought using bind() with non-root user doesn't do that,
but need to check the implementation in the kernel.

> Of course you could write a wrapper like the following to avoid that
> issue, but in my opinion it would be nice to have the option to get
> non-root ports directly from the library.

> /* Wrapper to to get a non-root port if necessary */
> static int get_port(uid_t uid)
> {
>         static int count = 10;
>         int port;

>         port = TST_GET_UNUSED_PORT(AF_INET, SOCK_STREAM);
>         if (port < 1000 && uid != 0) {
>                 if (!count)
>                         tst_brk(TBROK, "Could not get fitting port");
>                 count--;
>                 return get_port(uid);
>         }
>         return port;
> }

> Kind regards,

> Christian


Kind regards,
Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] TST_GET_UNUSED_PORT returns ports < 1024
  2019-05-29  9:33 [LTP] TST_GET_UNUSED_PORT returns ports < 1024 Christian Amann
  2019-05-30 19:00 ` Petr Vorel
@ 2019-06-06 13:43 ` Cyril Hrubis
  2019-06-06 14:45   ` Petr Vorel
  1 sibling, 1 reply; 5+ messages in thread
From: Cyril Hrubis @ 2019-06-06 13:43 UTC (permalink / raw)
  To: ltp

Hi!
> when using the TST_GET_UNUSED_PORT macro you sometimes get ports lower
> than 1024 which would require a testcase to have the
> CAP_NET_BIND_SERVICE capability (or simply run as root).

Looking at the code as far as I can tell the function returns the port
in the network endianity, which is big endian. Intel CPUs are little
endian, so if you want to print the port you actually have to use
ntohs() function to convert it to the host endianity. And if you are
passing that value in the sockaddr_in structure you must not use the
htons() since the value is already in the correct byte order. And yes
this is horribly confusing, but that's how it is.

I guess that we should write down this piece of information in the
documentation, because it looks like the tst_get_unused_port shell
helper does this incorrectly and prints the raw value instead of
converting it with ntohs().

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] TST_GET_UNUSED_PORT returns ports < 1024
  2019-06-06 13:43 ` Cyril Hrubis
@ 2019-06-06 14:45   ` Petr Vorel
  2019-06-07  6:21     ` Christian Amann
  0 siblings, 1 reply; 5+ messages in thread
From: Petr Vorel @ 2019-06-06 14:45 UTC (permalink / raw)
  To: ltp

Hi Cyril,

> > when using the TST_GET_UNUSED_PORT macro you sometimes get ports lower
> > than 1024 which would require a testcase to have the
> > CAP_NET_BIND_SERVICE capability (or simply run as root).

> Looking at the code as far as I can tell the function returns the port
> in the network endianity, which is big endian. Intel CPUs are little
> endian, so if you want to print the port you actually have to use
> ntohs() function to convert it to the host endianity. And if you are
> passing that value in the sockaddr_in structure you must not use the
> htons() since the value is already in the correct byte order. And yes
> this is horribly confusing, but that's how it is.
Thanks for debugging the problem.

> I guess that we should write down this piece of information in the
> documentation, because it looks like the tst_get_unused_port shell
> helper does this incorrectly and prints the raw value instead of
> converting it with ntohs().
Correct, I've sent a patch fixing it for shell tests [1].
As I noted there, this problem was even on version for old API.

Not sure about docs as there is no docs for network API shell/C functions yet
But even the problem for shell was fixed by that patch it'd be worth to add note
about byte order to C code tst_get_unused_port() and/or header defining
TST_GET_UNUSED_PORT() developers using it in C.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/patch/1111167/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] TST_GET_UNUSED_PORT returns ports < 1024
  2019-06-06 14:45   ` Petr Vorel
@ 2019-06-07  6:21     ` Christian Amann
  0 siblings, 0 replies; 5+ messages in thread
From: Christian Amann @ 2019-06-07  6:21 UTC (permalink / raw)
  To: ltp

Hi,

On 06/06/2019 16:45, Petr Vorel wrote:
> Hi Cyril,
>
>>> when using the TST_GET_UNUSED_PORT macro you sometimes get ports lower
>>> than 1024 which would require a testcase to have the
>>> CAP_NET_BIND_SERVICE capability (or simply run as root).
>> Looking at the code as far as I can tell the function returns the port
>> in the network endianity, which is big endian. Intel CPUs are little
>> endian, so if you want to print the port you actually have to use
>> ntohs() function to convert it to the host endianity. And if you are
>> passing that value in the sockaddr_in structure you must not use the
>> htons() since the value is already in the correct byte order. And yes
>> this is horribly confusing, but that's how it is.
> Thanks for debugging the problem.
>
Yes, and also thanks for the clarification.
>> I guess that we should write down this piece of information in the
>> documentation, because it looks like the tst_get_unused_port shell
>> helper does this incorrectly and prints the raw value instead of
>> converting it with ntohs().
> Correct, I've sent a patch fixing it for shell tests [1].
> As I noted there, this problem was even on version for old API.
>
> Not sure about docs as there is no docs for network API shell/C functions yet
> But even the problem for shell was fixed by that patch it'd be worth to add note
> about byte order to C code tst_get_unused_port() and/or header defining
> TST_GET_UNUSED_PORT() developers using it in C.

I agree. From what I can tell there is no easy way to quickly determine
the endianness. A little hint would be nice to cut down development time
(and possibly time spent debugging later on).

> Kind regards,
> Petr
>
> [1] https://patchwork.ozlabs.org/patch/1111167/

Kind regards,

Christian



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-06-07  6:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  9:33 [LTP] TST_GET_UNUSED_PORT returns ports < 1024 Christian Amann
2019-05-30 19:00 ` Petr Vorel
2019-06-06 13:43 ` Cyril Hrubis
2019-06-06 14:45   ` Petr Vorel
2019-06-07  6:21     ` Christian Amann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.