You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/10/05 16:45:52 UTC
[GitHub] [incubator-nuttx-apps] patacongo opened a new pull request #418: Incorporate IP configuration files into netinit.
patacongo opened a new pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418
## Summary
Uses logic from apps/fsutils/ipcfg to obtain the IP address configuration from a file, if available. This only effects the behavior of netinit if CONFIG_FSUTILS_IPCFG is selected and if an IP configuration file is available on a file system. Otherwise, it reverts to the preceding behavior.
Also adds definitions to apps/include/fsutils/ipcfg.h to formalize a bit encoding of the prototype which was already in use, but not documented.
## Impact
Obviously, the apps/netutils/netinit logic is affected. The use of the ipcfg IP configuration file is conditional so, if all was done right, there should be no impact.
## Testing
No testing has been performed yet. I would like to bet feedback first. It will take some effort to set up a configuration that has a IP configuration file so it would be preferable to make sure that I am verifying an acceptable design.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500291336
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
#else
- addr.s_addr = 0;
+ if (ret >= 0 && IPCFG_HAVE_STATIC(ipv4cfg.proto))
#endif
- netlib_set_ipv4addr(NET_DEVNAME, &addr);
+ {
+ /* Check if we are using DHCPC */
- /* Set up the default router address */
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (IPCFG_USE_DHCPC(ipv4cfg.proto))
+ {
+ g_use_dhcpc = true;
+ addr.s_addr = 0;
+ }
+ else
+#endif
+ {
+ /* We are not using DHCPC. We need an IP address */
- addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
- netlib_set_dripv4addr(NET_DEVNAME, &addr);
+#ifdef CONFIG_NETINIT_IPADDR
+ /* Check if we have a static IP address in the configuration file */
- /* Setup the subnet mask */
+ if (ipv4cfg.proto != 0)
Review comment:
The test` if (ipv4cfg.proto != 0)` is equivalent but a little stronger. If the STATIC bit is not set, the addresses should all be zero, but not necessarily.
But I think you are right, better to test the static bit because if the user wants to force a zero address I suppose that should be permitted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo edited a comment on pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#issuecomment-704362629
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500300394
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
#else
- addr.s_addr = 0;
+ if (ret >= 0 && IPCFG_HAVE_STATIC(ipv4cfg.proto))
#endif
- netlib_set_ipv4addr(NET_DEVNAME, &addr);
+ {
+ /* Check if we are using DHCPC */
- /* Set up the default router address */
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (IPCFG_USE_DHCPC(ipv4cfg.proto))
+ {
+ g_use_dhcpc = true;
+ addr.s_addr = 0;
+ }
+ else
+#endif
+ {
+ /* We are not using DHCPC. We need an IP address */
- addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
- netlib_set_dripv4addr(NET_DEVNAME, &addr);
+#ifdef CONFIG_NETINIT_IPADDR
+ /* Check if we have a static IP address in the configuration file */
- /* Setup the subnet mask */
+ if (ipv4cfg.proto != 0)
+ {
+ addr.s_addr = ipv4cfg.ipaddr;
+ }
+ else
+ {
+ addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ }
+#else
+ /* Use whatever was provided in the file (might be zero) */
- addr.s_addr = HTONL(CONFIG_NETINIT_NETMASK);
- netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ addr.s_addr = ipv4cfg.ipaddr;
#endif
+ }
-#ifdef CONFIG_NET_IPv6
+ netlib_set_ipv4addr(NET_DEVNAME, &addr);
+
+ /* Set up the remaining addresses */
+
+ if (IPCFG_HAVE_STATIC(ipv4cfg.proto))
+ {
+ /* Set up the default router address */
+
+ addr.s_addr = ipv4cfg.router;
+ netlib_set_dripv4addr(NET_DEVNAME, &addr);
+
+ /* Setup the subnet mask */
+
+ addr.s_addr = ipv4cfg.netmask;
+ netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ }
+
+#ifdef CONFIG_NETUTILS_DHCPC
+ /* No static addresses? That is fine if we are have addresses
+ * provided by the configuration, or if we are using DHCP.
+ */
+
+ else if (g_use_dhcpc)
+ {
+ /* Set up the default router address and sub-net mask */
+
+ addr.s_addr = 0;
+ netlib_set_dripv4addr(NET_DEVNAME, &addr);
+ netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ }
+#endif
+ else
+ {
+ /* Otherwise, set up the configured default router address */
+
+ addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
Review comment:
It is, but sometimes a small code duplication can save a lot of complexity. I don't know how to do the switch statement you are referring to.
The complexity is due to:
- CONFIG_NETUTILS_DHCPC meas that the DHCPC code has been built into the system and is available.
DHCPC may used under the following conditions if CONFIG_NETUTILS_DHCPC=y:
- CONFIG_FSUTILS_IPCFG is selected, a configuration file is read, and the boot protocol if DHCPC
- CONFIG_FSUTILS_IPCFG is NOT selected or a configuration file is no available and CONFIG_NETINIT_DHCPC is selected.
There are lots of ways that there can be mismatches and the complex logic tries to resolve the complexities. For example, if the configuration file asks for DHCPC but CONFIG_NETUTILS_DHCPC is not selected.
A little redundancy is a good thing if it gets us out of some of the complexity.
You comment is noted, but I don't see anything actionable here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#issuecomment-704362629
I have finished verification of this change. Looks good.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] davids5 commented on pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#issuecomment-703796470
@patacongo As soon as I can I will take a look.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500286471
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
Review comment:
Resolving this conversation because the question, while important, does not apply to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500286027
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
Review comment:
No, that is impossible in a modular, layered architecture. User-space applications must be completely ignorant of the underlying media and hence must never make any assumptions about media properties, whether or not it is eraseable or what the erased value it. Reading from an erased FLASH would fail for the base the binary file: The address family of 0xffff would be invalid; it would not match AF_INET or AF_INET6 The hdr.next field would be 0xff so that might go off into never-never-land. I suppose there should be a sanity check on the hdr.next field too.
I think your PR #419 addresses the versioning issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500293426
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
#else
- addr.s_addr = 0;
+ if (ret >= 0 && IPCFG_HAVE_STATIC(ipv4cfg.proto))
#endif
- netlib_set_ipv4addr(NET_DEVNAME, &addr);
+ {
+ /* Check if we are using DHCPC */
- /* Set up the default router address */
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (IPCFG_USE_DHCPC(ipv4cfg.proto))
+ {
+ g_use_dhcpc = true;
+ addr.s_addr = 0;
+ }
+ else
+#endif
+ {
+ /* We are not using DHCPC. We need an IP address */
- addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
- netlib_set_dripv4addr(NET_DEVNAME, &addr);
+#ifdef CONFIG_NETINIT_IPADDR
+ /* Check if we have a static IP address in the configuration file */
- /* Setup the subnet mask */
+ if (ipv4cfg.proto != 0)
Review comment:
Changes per your suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] davids5 commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500216142
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
Review comment:
Consider combining the adjacent #ifdefs
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
#else
- addr.s_addr = 0;
+ if (ret >= 0 && IPCFG_HAVE_STATIC(ipv4cfg.proto))
#endif
- netlib_set_ipv4addr(NET_DEVNAME, &addr);
+ {
+ /* Check if we are using DHCPC */
- /* Set up the default router address */
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (IPCFG_USE_DHCPC(ipv4cfg.proto))
+ {
+ g_use_dhcpc = true;
+ addr.s_addr = 0;
+ }
+ else
+#endif
+ {
+ /* We are not using DHCPC. We need an IP address */
- addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
- netlib_set_dripv4addr(NET_DEVNAME, &addr);
+#ifdef CONFIG_NETINIT_IPADDR
+ /* Check if we have a static IP address in the configuration file */
- /* Setup the subnet mask */
+ if (ipv4cfg.proto != 0)
Review comment:
in the DHCP case the `else` limits the scope but would it it make sense to check the static bit here?
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
Review comment:
Side comment: Is ipcfg_read checking for erased values of 0xff and the to come build version vs read version.?
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
#else
- addr.s_addr = 0;
+ if (ret >= 0 && IPCFG_HAVE_STATIC(ipv4cfg.proto))
#endif
- netlib_set_ipv4addr(NET_DEVNAME, &addr);
+ {
+ /* Check if we are using DHCPC */
- /* Set up the default router address */
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (IPCFG_USE_DHCPC(ipv4cfg.proto))
+ {
+ g_use_dhcpc = true;
+ addr.s_addr = 0;
+ }
+ else
+#endif
+ {
+ /* We are not using DHCPC. We need an IP address */
- addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
- netlib_set_dripv4addr(NET_DEVNAME, &addr);
+#ifdef CONFIG_NETINIT_IPADDR
+ /* Check if we have a static IP address in the configuration file */
- /* Setup the subnet mask */
+ if (ipv4cfg.proto != 0)
+ {
+ addr.s_addr = ipv4cfg.ipaddr;
+ }
+ else
+ {
+ addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ }
+#else
+ /* Use whatever was provided in the file (might be zero) */
- addr.s_addr = HTONL(CONFIG_NETINIT_NETMASK);
- netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ addr.s_addr = ipv4cfg.ipaddr;
#endif
+ }
-#ifdef CONFIG_NET_IPv6
+ netlib_set_ipv4addr(NET_DEVNAME, &addr);
+
+ /* Set up the remaining addresses */
+
+ if (IPCFG_HAVE_STATIC(ipv4cfg.proto))
+ {
+ /* Set up the default router address */
+
+ addr.s_addr = ipv4cfg.router;
+ netlib_set_dripv4addr(NET_DEVNAME, &addr);
+
+ /* Setup the subnet mask */
+
+ addr.s_addr = ipv4cfg.netmask;
+ netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ }
+
+#ifdef CONFIG_NETUTILS_DHCPC
+ /* No static addresses? That is fine if we are have addresses
+ * provided by the configuration, or if we are using DHCP.
+ */
+
+ else if (g_use_dhcpc)
+ {
+ /* Set up the default router address and sub-net mask */
+
+ addr.s_addr = 0;
+ netlib_set_dripv4addr(NET_DEVNAME, &addr);
+ netlib_set_ipv4netmask(NET_DEVNAME, &addr);
+ }
+#endif
+ else
+ {
+ /* Otherwise, set up the configured default router address */
+
+ addr.s_addr = HTONL(CONFIG_NETINIT_DRIPADDR);
Review comment:
This case seems redundant to the one on line 411. But I am having a super hard time with all the #ifdef and 'else' and maintaining visual alignment. - that might just be my deficit. Which is fine. But I cannot help I wonder if it would be better from a readability stand point to assemble a state variable and have switch statement ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] davids5 commented on pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#issuecomment-704439286
@patacongo - thank you for this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500280302
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
Review comment:
Done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] davids5 commented on a change in pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#discussion_r500293343
##########
File path: netutils/netinit/netinit.c
##########
@@ -344,66 +340,239 @@ static void netinit_set_macaddr(void)
#endif
/****************************************************************************
- * Name: netinit_set_ipaddrs
+ * Name: netinit_set_ipv4addrs
*
* Description:
- * Setup IP addresses.
- *
- * For 6LoWPAN, the IP address derives from the MAC address. Setting it
- * to any user provided value is asking for trouble.
+ * Setup IPv4 addresses.
*
****************************************************************************/
#if defined(NETINIT_HAVE_NETDEV) && !defined(CONFIG_NET_6LOWPAN) && ! \
- defined(CONFIG_NET_IEEE802154)
-static void netinit_set_ipaddrs(void)
+ defined(CONFIG_NET_IEEE802154) && defined(CONFIG_NET_IPv4)
+static inline void netinit_set_ipv4addrs(void)
{
-#ifdef CONFIG_NET_IPv4
struct in_addr addr;
+#ifdef CONFIG_FSUTILS_IPCFG
+ struct ipv4cfg_s ipv4cfg;
+ int ret;
+#endif
- /* Set up our host address */
+#ifdef CONFIG_FSUTILS_IPCFG
+ /* Attempt to obtain IPv4 address configuration from the IP configuration
+ * file.
+ */
-#ifndef CONFIG_NETINIT_DHCPC
- addr.s_addr = HTONL(CONFIG_NETINIT_IPADDR);
+ ret = ipcfg_read(NET_DEVNAME, (FAR struct ipcfg_s *)&ipv4cfg, AF_INET);
+#ifdef CONFIG_NETUTILS_DHCPC
+ if (ret >= 0 && ipv4cfg.proto != IPv4PROTO_NONE)
Review comment:
> he hdr.next field would be 0xff so that might go off
yes - the version, can catch this. May be it should be checked first.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418#issuecomment-704472277
> @patacongo - thank you for this!
You are welcome. I am of the belief that if you are going to bitch and moan about someone else's solution, then it is your responsibility to provide a better one. My complaints about boardctl() cost me several days of effort!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] davids5 merged pull request #418: Incorporate IP configuration files into netinit.
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #418:
URL: https://github.com/apache/incubator-nuttx-apps/pull/418
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org