[SLOF] [PATCH v3 2/3] ping: add netmask in the ping argument
Thomas Huth
thuth at redhat.com
Tue May 3 17:43:19 AEST 2016
On 03.05.2016 06:01, Nikunj A Dadhania wrote:
> The current ping command does not take netmask as argument, updated the
> ping command to take "client-ip/nn" format ip address.
>
> Add routine to return netmask(class based), when not provided by user.
>
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
> clients/net-snk/app/netapps/args.c | 44 ++++++++++++++++++++++++++++++++++----
> clients/net-snk/app/netapps/args.h | 1 +
> clients/net-snk/app/netapps/ping.c | 21 ++++++++++++++++--
> clients/net-snk/app/netlib/ipv4.c | 21 ++++++++++++++++++
> clients/net-snk/app/netlib/ipv4.h | 1 +
> slof/fs/loaders.fs | 4 ++--
> 6 files changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/clients/net-snk/app/netapps/args.c b/clients/net-snk/app/netapps/args.c
> index 52215e6..ed25c7c 100644
> --- a/clients/net-snk/app/netapps/args.c
> +++ b/clients/net-snk/app/netapps/args.c
> @@ -98,20 +98,22 @@ argncpy(const char *arg_str, unsigned int index, char *buffer,
> }
>
> /**
> - * Converts "255.255.255.255" -> char[4] = { 0xff, 0xff, 0xff, 0xff }
> + * Converts "255.255.255.255/nn" -> char[4] = { 0xff, 0xff, 0xff, 0xff }
> + * *netmask = subnet_netmask(nn)
> *
> * @param str string to be converted
> * @param ip in case of SUCCESS - 32-bit long IP
> - in case of FAULT - zero
> + * in case of FAULT - zero
> + * @param netmask return netmask if there is a valid /nn encoding in IP
> * @return TRUE - IP converted successfully;
> * FALSE - error condition occurs (e.g. bad format)
> */
> int
> -strtoip(const char *str, char ip[4])
> +strtoip_netmask(const char *str, char ip[4], unsigned int *netmask)
> {
> char octet[10];
> int res;
> - unsigned int i = 0, len;
> + unsigned int i = 0, len, has_nn = 0;
>
> while (*str != 0) {
> if (i > 3 || !isdigit(*str))
> @@ -123,6 +125,14 @@ strtoip(const char *str, char ip[4])
> strncpy(octet, str, len);
> octet[len] = 0;
> str += len;
> + } else if (strstr(str, "\\") != NULL) {
> + len = (short) (strstr(str, "\\") - str);
Shouldn't that be a forward slash instead? Or will this cause trouble
with the slash handling from the Forth code? (in the latter case, you
should at least adapt the 'Converts "255.255.255.255/nn"' comment in
front of this function, I think)
Anyway, you could also use strchr() instead of strstr() as a small
optimization, I think.
> + if (len >= 10)
> + return 0;
> + strncpy(octet, str, len);
> + octet[len] = 0;
> + str += len;
> + has_nn = 1;
> } else {
> strncpy(octet, str, 9);
> octet[9] = 0;
> @@ -135,9 +145,35 @@ strtoip(const char *str, char ip[4])
> i++;
> if (*str == '.')
> str++;
> + if(has_nn) {
> + str++;
> + strncpy(octet, str, 9);
> + octet[9] = 0;
> + res = strtol(octet, NULL, 10);
> + str += strlen(octet);
> + if (res > 31 || res < 1)
> + return 0;
> + if (netmask)
> + *netmask = (0xFFFFFFFFUL << (32 - res)) & 0xFFFFFFFFUL;
I think you could simply do without the final "& 0xFFFFFFFFUL" here.
And for the first 0xFFFFFFFF, you also don't need the UL suffix, do you?
> + }
> }
>
> if (i != 4)
> return 0;
> return -1;
> }
> +
> +/**
> + * Converts "255.255.255.255" -> char[4] = { 0xff, 0xff, 0xff, 0xff }
> + *
> + * @param str string to be converted
> + * @param ip in case of SUCCESS - 32-bit long IP
> + in case of FAULT - zero
> + * @return TRUE - IP converted successfully;
> + * FALSE - error condition occurs (e.g. bad format)
> + */
> +int
> +strtoip(const char *str, char ip[4])
> +{
> + return strtoip_netmask(str, ip, NULL);
> +}
> diff --git a/clients/net-snk/app/netapps/args.h b/clients/net-snk/app/netapps/args.h
> index b80982a..1ede9a8 100644
> --- a/clients/net-snk/app/netapps/args.h
> +++ b/clients/net-snk/app/netapps/args.h
> @@ -18,5 +18,6 @@ unsigned int get_args_count(const char *);
> unsigned int get_arg_length(const char *);
> char *argncpy(const char *, unsigned int, char *, unsigned int);
> int strtoip(const char *, char[4]);
> +int strtoip_netmask(const char *, char[4], unsigned int *netmask);
>
> #endif /* _ARGS_H */
> diff --git a/clients/net-snk/app/netapps/ping.c b/clients/net-snk/app/netapps/ping.c
> index 4facf06..bd8f2d3 100644
> --- a/clients/net-snk/app/netapps/ping.c
> +++ b/clients/net-snk/app/netapps/ping.c
> @@ -35,13 +35,15 @@ struct ping_args {
> unsigned int integer;
> } gateway_ip;
> unsigned int timeout;
> + /* Netmask 192.168.1.10/24 format */
What do you mean with "192.168.1.10/24 format" here? You store here the
real netmask, not the prefix length, right?
> + unsigned int netmask;
> };
>
> static void
> usage(void)
> {
> printf
> - ("\nping device-path:[device-args,]server-ip,[client-ip],[gateway-ip][,timeout]\n");
> + ("\nping device-path:[device-args,]server-ip,[client-ip[\\nn]],[gateway-ip][,timeout]\n");
> }
>
> @@ -82,7 +84,7 @@ parse_args(const char *args, struct ping_args *ping_args)
> }
>
> argncpy(args, 0, buf, 64);
> - if (!strtoip(buf, ping_args->client_ip.string)) {
> + if (!strtoip_netmask(buf, ping_args->client_ip.string, &ping_args->netmask)) {
> /* this should have been the client (our) IP address */
> return -1;
> } else {
> @@ -112,6 +114,7 @@ ping(int argc, char *argv[])
> int fd_device;
> struct ping_args ping_args;
> uint8_t own_mac[6];
> + uint32_t netmask;
>
> memset(&ping_args, 0, sizeof(struct ping_args));
>
> @@ -163,6 +166,14 @@ ping(int argc, char *argv[])
>
> } else {
> memcpy(&fn_ip.own_ip, &ping_args.client_ip.integer, 4);
> + if (!ping_args.netmask) {
> + /* Netmask is not provided, assume default according to
> + * the network class
> + */
> + ping_args.netmask = get_default_ipv4_netmask(ping_args.client_ip.string);
> + }
> + set_ipv4_netmask(ping_args.netmask);
> +
> arp_failed = 1;
> printf(" Own IP address: ");
> }
> @@ -174,6 +185,12 @@ ping(int argc, char *argv[])
> ((fn_ip.own_ip >> 24) & 0xFF), ((fn_ip.own_ip >> 16) & 0xFF),
> ((fn_ip.own_ip >> 8) & 0xFF), (fn_ip.own_ip & 0xFF));
>
> + if ((netmask = get_ipv4_netmask())) {
Just a matter of taste - but could you maybe write that in two lines:
netmask = get_ipv4_netmask();
if (netmask) ...
> + printf(" Netmask : ");
> + printf("%d.%d.%d.%d\n", ((netmask >> 24) & 0xFF), ((netmask >> 16) & 0xFF),
> + ((netmask >> 8) & 0xFF), (netmask & 0xFF));
> + }
> +
> memcpy(&fn_ip.server_ip, &ping_args.server_ip.integer, 4);
> printf(" Ping to %d.%d.%d.%d ", ((fn_ip.server_ip >> 24) & 0xFF),
> ((fn_ip.server_ip >> 16) & 0xFF),
> diff --git a/clients/net-snk/app/netlib/ipv4.c b/clients/net-snk/app/netlib/ipv4.c
> index 2b92c77..3a1a789 100644
> --- a/clients/net-snk/app/netlib/ipv4.c
> +++ b/clients/net-snk/app/netlib/ipv4.c
> @@ -250,6 +250,27 @@ uint32_t get_ipv4_netmask(void)
> }
>
> /**
> + * IPv4: Get the default subnet mask according to the IP class
> + *
> + * @param ip_addr IPv4 address
> + * @return default netmask according to the IP class
> + */
> +uint32_t get_default_ipv4_netmask(char *ip_addr)
> +{
> + unsigned char top;
> +
> + top = ip_addr[0];
> + if (top > 0 && top < 128)
> + return 0xFF000000; /* Class A: 255.0.0.0 */
> + else if (top >= 128 && top < 192)
> + return 0xFFFF0000; /* Class B: 255.255.0.0 */
> + else if (top >= 192 && top < 224)
> + return 0xFFFFFF00; /* Class C: 255.255.255.0 */
> + else
> + return 0;
> +}
> +
> +/**
> * IPv4: Creates IP-packet. Places IP-header in a packet and fills it
> * with corresponding information.
> * <p>
> diff --git a/clients/net-snk/app/netlib/ipv4.h b/clients/net-snk/app/netlib/ipv4.h
> index 18821ea..5717c9a 100644
> --- a/clients/net-snk/app/netlib/ipv4.h
> +++ b/clients/net-snk/app/netlib/ipv4.h
> @@ -69,6 +69,7 @@ extern void set_ipv4_router(uint32_t router_ip);
> extern uint32_t get_ipv4_router(void);
> extern void set_ipv4_netmask(uint32_t subnet_mask);
> extern uint32_t get_ipv4_netmask(void);
> +extern uint32_t get_default_ipv4_netmask(char *ip_addr);
>
> extern int (*send_ip) (int fd, void *, int);
>
> diff --git a/slof/fs/loaders.fs b/slof/fs/loaders.fs
> index 1478c70..8ae502d 100644
> --- a/slof/fs/loaders.fs
> +++ b/slof/fs/loaders.fs
> @@ -70,7 +70,7 @@ CREATE load-list 2 cells allot load-list 2 cells erase
> THEN
> ;
>
> -: ping ( "{device-path:[device-args,]server-ip,[client-ip],[gateway-ip][,timeout]}" -- )
> +: ping ( "{device-path:[device-args,]server-ip,[client-ip[\nn]],[gateway-ip][,timeout]}" -- )
> my-self >r current-node @ >r \ Save my-self
> (parse-line) open-dev dup IF
> dup to my-self dup ihandle>phandle set-node
> @@ -83,7 +83,7 @@ CREATE load-list 2 cells allot load-list 2 cells erase
> swap close-dev
> ELSE
> cr
> - ." Usage: ping device-path:[device-args,]server-ip,[client-ip],[gateway-ip][,timeout]"
> + ." Usage: ping device-path:[device-args,]server-ip,[client-ip[\nn]],[gateway-ip][,timeout]"
> cr drop
> THEN
> r> set-node r> to my-self \ Restore my-self
>
Apart from the nits, the patch looks fine to me.
Thomas
More information about the SLOF
mailing list