Skip to content

netutils/ping: improve ping and ping6 for get_host error handling #3404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
masc2008 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

netutils/ping: improve ping and ping6 for get_host error handling #3404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 1 addition & 46 deletions include/netutils/icmp_ping.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
****************************************************************************/

#include <netinet/in.h>
#include "icmp_pub.h"

/****************************************************************************
* Pre-processor Definitions
Expand All @@ -42,7 +43,6 @@

/* Negative odd number represent error(unrecoverable) */

#define ICMP_E_HOSTIP -1 /* extra: not used */
#define ICMP_E_MEMORY -3 /* extra: not used */
#define ICMP_E_SOCKET -5 /* extra: error code */
#define ICMP_E_SENDTO -7 /* extra: error code */
Expand All @@ -67,53 +67,8 @@
* Public Types
****************************************************************************/

struct ping_result_s;

struct ping_info_s
{
FAR const char *hostname; /* Host name to ping */
#ifdef CONFIG_NET_BINDTODEVICE
FAR const char *devname; /* Device name to bind */
#endif
uint16_t count; /* Number of pings requested */
uint16_t datalen; /* Number of bytes to be sent */
uint16_t delay; /* Deciseconds to delay between pings */
uint16_t timeout; /* Deciseconds to wait response before timeout */
FAR void *priv; /* Private context for callback */
void (*callback)(FAR const struct ping_result_s *result);
};

struct ping_result_s
{
int code; /* Notice code ICMP_I/E/W_XXX */
long extra; /* Extra information for code */
struct in_addr dest; /* Target address to ping */
uint16_t nrequests; /* Number of ICMP ECHO requests sent */
uint16_t nreplies; /* Number of matching ICMP ECHO replies received */
uint16_t outsize; /* Bytes(include ICMP header) to be sent */
uint16_t id; /* ICMP_ECHO id */
uint16_t seqno; /* ICMP_ECHO seqno */
FAR const struct ping_info_s *info;
};

/****************************************************************************
* Public Function Prototypes
****************************************************************************/

#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif

void icmp_ping(FAR const struct ping_info_s *info);

#undef EXTERN
#ifdef __cplusplus
}
#endif

#endif /* __APPS_INCLUDE_NETUTILS_ICMP_PING_H */
82 changes: 82 additions & 0 deletions include/netutils/icmp_pub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/****************************************************************************
* apps/include/netutils/icmp_pub.h
*
* SPDX-License-Identifier: Apache-2.0
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/

#ifndef __APPS_INCLUDE_NETUTILS_ICMP_PUB_H
#define __APPS_INCLUDE_NETUTILS_ICMP_PUB_H

#define ICMP_E_HOSTIP -1 /* extra: not used */

struct ping_result_s;

struct ping_info_s
{
FAR const char *hostname; /* Host name to ping */
#ifdef CONFIG_NET_BINDTODEVICE
FAR const char *devname; /* Device name to bind */
#endif
uint16_t flag; /* v4 or v6 */
uint16_t count; /* Number of pings requested */
uint16_t datalen; /* Number of bytes to be sent */
uint16_t delay; /* Deciseconds to delay between pings */
uint16_t timeout; /* Deciseconds to wait response before timeout */
FAR void *priv; /* Private context for callback */
void (*callback)(FAR const struct ping_result_s *result);
};

struct ping_result_s
{
int code; /* Notice code ICMPv6_I/E/W_XXX */
long extra; /* Extra information for code */
union
{
#ifdef CONFIG_SYSTEM_PING
struct in_addr v4; /* Target address to ping */
#endif
#ifdef CONFIG_SYSTEM_PING6
struct in6_addr v6; /* Target address to ping */
#endif
} dest;
uint16_t nrequests; /* Number of ICMP ECHO requests sent */
uint16_t nreplies; /* Number of matching ICMP ECHO replies received */
uint16_t outsize; /* Bytes(include ICMP header) to be sent */
uint16_t id; /* ICMPv6_ECHO id */
uint16_t seqno; /* ICMPv6_ECHO seqno */
FAR const struct ping_info_s *info;
};

#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif

void icmp_ping(FAR const struct ping_info_s *info);
void icmp6_ping(FAR const struct ping_info_s *info);

#undef EXTERN
#ifdef __cplusplus
}
#endif
#endif /* __APPS_INCLUDE_NETUTILS_ICMP_PUB_H */
47 changes: 1 addition & 46 deletions include/netutils/icmpv6_ping.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
****************************************************************************/

#include <netinet/in.h>
#include "icmp_pub.h"

/****************************************************************************
* Pre-processor Definitions
Expand All @@ -42,7 +43,6 @@

/* Negative odd number represent error(unrecoverable) */

#define ICMPv6_E_HOSTIP -1 /* extra: not used */
#define ICMPv6_E_MEMORY -3 /* extra: not used */
#define ICMPv6_E_SOCKET -5 /* extra: error code */
#define ICMPv6_E_SENDTO -7 /* extra: error code */
Expand All @@ -66,53 +66,8 @@
* Public Types
****************************************************************************/

struct ping6_result_s;

struct ping6_info_s
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move the reorg change to new pr

Copy link
Contributor Author

@masc2008 masc2008 Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose is to let ping_result_s and ping6_result_s share same data structure. since in the callback same pointer was transferred, they will be used in different callback. same reason to ping_info_s and ping6_info_s.
if they have have private structure, and changed privately, then it will have very high risk, not easy to maintain them. considering this, should let them share same.

Copy link
Contributor Author

@masc2008 masc2008 Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key change is here: others are for this:
image

Copy link
Contributor Author

@masc2008 masc2008 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiang781216 , any further concern?

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping and ping6 is totally seperated in the old implementation, so I would suggest that:

  1. Keep the origin design, and change the above code to posix_spawn
  2. Implement ipv4/ipv6 ping in one file

either is better than the current change which couple ping/ping6 in adhoc way.

Copy link
Contributor Author

@masc2008 masc2008 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say ipv4/ipv6 ping should be in one file, and that is correct way. if choose that direction: FreeBSD version of ping can match the requirement, so why do we need do such basic code again? what I did is some work with accept able cost and fully leverage the current file. if re-org based current ping code, it will a new folder with some repeated data structures. this task has afflicted me too long. I would rather give up...

Copy link
Contributor Author

@masc2008 masc2008 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say ipv4/ipv6 ping should be in one file, and that is correct way. if choose that direction: FreeBSD version of ping can match the requirement, so why do we need do such basic code again? what I did is some work with accept able cost and fully leverage the current file. if re-org based current ping code, it will a new folder with some repeated data structures. this task has afflicted me too long. I would rather give up...

Copy link
Contributor Author

@masc2008 masc2008 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say ipv4/ipv6 ping should be in one file, and that is correct way. if choose that direction: FreeBSD version of ping can match the requirement, so why do we need do such basic code again? what I did is some work with accept able cost and fully leverage the current file. if re-org based current ping code, it will a new folder with some repeated data structures. this task has afflicted me too long. I would rather give up...

{
FAR const char *hostname; /* Host name to ping */
#ifdef CONFIG_NET_BINDTODEVICE
FAR const char *devname; /* Device name to bind */
#endif
uint16_t count; /* Number of pings requested */
uint16_t datalen; /* Number of bytes to be sent */
uint16_t delay; /* Deciseconds to delay between pings */
uint16_t timeout; /* Deciseconds to wait response before timeout */
FAR void *priv; /* Private context for callback */
void (*callback)(FAR const struct ping6_result_s *result);
};

struct ping6_result_s
{
int code; /* Notice code ICMPv6_I/E/W_XXX */
long extra; /* Extra information for code */
struct in6_addr dest; /* Target address to ping */
uint16_t nrequests; /* Number of ICMP ECHO requests sent */
uint16_t nreplies; /* Number of matching ICMP ECHO replies received */
uint16_t outsize; /* Bytes(include ICMP header) to be sent */
uint16_t id; /* ICMPv6_ECHO id */
uint16_t seqno; /* ICMPv6_ECHO seqno */
FAR const struct ping6_info_s *info;
};

/****************************************************************************
* Public Function Prototypes
****************************************************************************/

#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif

void icmp6_ping(FAR const struct ping6_info_s *info);

#undef EXTERN
#ifdef __cplusplus
}
#endif

#endif /* __APPS_INCLUDE_NETUTILS_ICMP_PING_H */
52 changes: 52 additions & 0 deletions include/netutils/ping_pub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/****************************************************************************
* apps/include/netutils/ping_pub.h
*
* SPDX-License-Identifier: Apache-2.0
*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership. The
* ASF licenses this file to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
****************************************************************************/

#ifndef __APPS_INCLUDE_NETUTILS_PING_PUB_H
#define __APPS_INCLUDE_NETUTILS_PING_PUB_H

struct ping_priv_s
{
int code; /* Notice code ICMP_I/E/W_XXX */
long tmin; /* Minimum round trip time */
long tmax; /* Maximum round trip time */
long long tsum; /* Sum of all times, for doing average */
long long tsum2; /* Sum2 is the sum of the squares of sum ,for doing mean deviation */
};

#undef EXTERN
#if defined(__cplusplus)
#define EXTERN extern "C"
extern "C"
{
#else
#define EXTERN extern
#endif

void ping_result(FAR const struct ping_result_s *result);
void ping6_result(FAR const struct ping_result_s *result);

#undef EXTERN
#ifdef __cplusplus
}
#endif

#endif /* __APPS_INCLUDE_NETUTILS_PING_PUB_H */
4 changes: 2 additions & 2 deletions netutils/ping/icmp_ping.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ void icmp_ping(FAR const struct ping_info_s *info)
result.info = info;
result.id = ping_newid();
result.outsize = ICMP_IOBUFFER_SIZE(info->datalen);
if (ping_gethostip(info->hostname, &result.dest) < 0)
if (ping_gethostip(info->hostname, &result.dest.v4) < 0)
{
icmp_callback(&result, ICMP_E_HOSTIP, 0);
return;
Expand Down Expand Up @@ -300,7 +300,7 @@ void icmp_ping(FAR const struct ping_info_s *info)
memset(&priv->destaddr, 0, sizeof(struct sockaddr_in));
priv->destaddr.sin_family = AF_INET;
priv->destaddr.sin_port = 0;
priv->destaddr.sin_addr.s_addr = result.dest.s_addr;
priv->destaddr.sin_addr.s_addr = result.dest.v4.s_addr;

icmp_callback(&result, ICMP_I_BEGIN, 0);

Expand Down
10 changes: 5 additions & 5 deletions netutils/ping/icmpv6_ping.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ static int ping6_gethostip(FAR const char *hostname,
* Name: icmp6_callback
****************************************************************************/

static void icmp6_callback(FAR struct ping6_result_s *result,
static void icmp6_callback(FAR struct ping_result_s *result,
int code, long extra)
{
result->code = code;
Expand All @@ -165,9 +165,9 @@ static void icmp6_callback(FAR struct ping6_result_s *result,
* Name: icmp6_ping
****************************************************************************/

void icmp6_ping(FAR const struct ping6_info_s *info)
void icmp6_ping(FAR const struct ping_info_s *info)
{
struct ping6_result_s result;
struct ping_result_s result;
struct sockaddr_in6 destaddr;
struct sockaddr_in6 fromaddr;
struct icmp6_filter filter;
Expand Down Expand Up @@ -197,9 +197,9 @@ void icmp6_ping(FAR const struct ping6_info_s *info)
result.info = info;
result.id = ping6_newid();
result.outsize = ICMPv6_IOBUFFER_SIZE(info->datalen);
if (ping6_gethostip(info->hostname, &result.dest) < 0)
if (ping6_gethostip(info->hostname, &result.dest.v6) < 0)
{
icmp6_callback(&result, ICMPv6_E_HOSTIP, 0);
icmp6_callback(&result, ICMP_E_HOSTIP, 0);
return;
}

Expand Down
Loading
Loading