You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "daniel-p-carvalho (via GitHub)" <gi...@apache.org> on 2026/10/18 18:24:38 UTC

[PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

daniel-p-carvalho opened a new pull request, #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876

   ## Summary
   
   Add the `wifibridge` command and the `netutils/wifibridge` library behind it. They turn a Wi-Fi interface into a SoftAP and bridge it to an Ethernet interface at layer 2, so Ethernet devices (printers, PLCs, PCs) reach the wireless network and the other way round, without routing. The bridge interface carries the IP address and, optionally, a DHCP server. It is the NuttX counterpart of Microchip's Ethernet to Wi-Fi bridge reference design for the EV49N51A, but it is not tied to a board: it works with any Ethernet and Wi-Fi netdev pair that can be bridge ports.
   
   ```
   wifibridge start [-s <ssid>] [-p <passphrase>] [-c <channel>] [-3] [-a <ipaddr>] [-d]
   wifibridge stop
   wifibridge status
   ```
   
   - `netutils/wifibridge` (`include/netutils/wifibridge.h`): `wifibridge_start()`, `wifibridge_stop()` and `wifibridge_status()`. `start` sets up the SoftAP with `wapi` (WPA2-PSK or WPA3-SAE, CCMP), creates the bridge with the `SIOCBR*` ioctl commands, adds the ports, sets the bridge address and optionally starts the DHCP server. If a step fails, what was done is undone.
   - `system/wifibridge`: the NSH command, with the defaults taken from Kconfig.
   - A README for the command. The documentation page is in apache/nuttx (see below).
   
   It builds on `CONFIG_NET_BRIDGE` and `brctl` (apache/nuttx#20519, nuttx-apps#3819), and needs a Wi-Fi driver with the master mode and an Ethernet driver that receives foreign MAC addresses (`CONFIG_NET_PROMISCUOUS`).
   
   Not in this PR: STA mode (a station cannot be bridged at layer 2), and saving the SSID and the passphrase.
   
   ## Impact
   
   New, optional (`CONFIG_SYSTEM_WIFIBRIDGE`, `CONFIG_NETUTILS_WIFIBRIDGE`), default `n`. No change to existing code, build or configurations. The default passphrase is documented as something to change before deploying.
   
   Companion changes in apache/nuttx, to be opened after this one: the documentation page `Documentation/applications/system/wifibridge`, and a PIC32MZ option for the Ethernet MAC address, since the PIC32MZ-W1 has no factory programmed one.
   
   ## Testing
   
   Host: Ubuntu Linux, XC32 v6.00 (`xc32-gcc`). Target: EV49N51A (PIC32MZ-W1), with a test configuration based on the board's bridge configuration plus `CONFIG_SYSTEM_WIFIBRIDGE` and `CONFIG_NETUTILS_WIFIBRIDGE_DHCPD` (not part of this PR). The Ethernet port is cabled to the host and a Wi-Fi station of the host joins the SoftAP.
   
   - `wifibridge start -d` brings up the SoftAP, `br0` (10.0.0.2) with `eth0` and `wlan0` up, and the DHCP server on `br0`.
   - The Wi-Fi station gets 10.0.0.100 from the board and pings `br0`, and the Ethernet device (10.0.0.101, in a network namespace so that the traffic has to cross the cable) through the bridge: 5/5 and 4/4, with the TX counters of `eth0` and `wlan0` increasing together.
   - Five `stop`/`start` cycles (and three more after the last cleanup): the access point stops and starts each time and the heap use stays at 27504 to 27520 bytes.
   - Error paths: `start` twice returns `EEXIST`, an invalid channel or a short passphrase returns `EINVAL`, `stop` without a bridge returns `ENODEV`.
   - Custom `-s`, `-p`, `-c` and `-a` values bring up the access point with those parameters.
   - `sim:nsh` with the options enabled builds without warnings. `checkpatch` passes.
   
   Not tested: WPA3 (`-3`), which needs the BA414E engine of the PIC32MZ-W1.
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4308642100


##########
netutils/wifibridge/Kconfig:
##########
@@ -0,0 +1,32 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config NETUTILS_WIFIBRIDGE
+	bool "Wi-Fi SoftAP to Ethernet bridge"
+	default n
+	depends on NET_IPv4 && NET_BRIDGE && WIRELESS_WAPI
+	select NETUTILS_NETLIB

Review Comment:
   depend on ?



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,378 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include <nuttx/net/ioctl.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_ifcmd
+ *
+ * Description:
+ *   Add or remove a bridge port.
+ *
+ ****************************************************************************/
+
+static int wifibridge_ifcmd(int sock, int cmd, FAR const char *bridge,
+                            FAR const char *ifname)
+{
+  struct ifreq req;
+
+  memset(&req, 0, sizeof(req));
+  strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+  req.ifr_ifindex = if_nametoindex(ifname);
+  if (req.ifr_ifindex == 0)
+    {
+      return -ENODEV;

Review Comment:
   -errno



##########
system/wifibridge/README.md:
##########
@@ -0,0 +1,44 @@
+# wifibridge
+
+Wi-Fi SoftAP to Ethernet bridge for NuttX.  It turns a Wi-Fi interface

Review Comment:
   move to nuttx/Documentation



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,378 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include <nuttx/net/ioctl.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_ifcmd
+ *
+ * Description:
+ *   Add or remove a bridge port.
+ *
+ ****************************************************************************/
+
+static int wifibridge_ifcmd(int sock, int cmd, FAR const char *bridge,

Review Comment:
   should we add the common bridge to netlib?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4313599871


##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;

Review Comment:
   sorroy, my mistake.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "daniel-p-carvalho (via GitHub)" <gi...@apache.org>.
daniel-p-carvalho commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4308829605


##########
netutils/wifibridge/Kconfig:
##########
@@ -0,0 +1,32 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config NETUTILS_WIFIBRIDGE
+	bool "Wi-Fi SoftAP to Ethernet bridge"
+	default n
+	depends on NET_IPv4 && NET_BRIDGE && WIRELESS_WAPI
+	select NETUTILS_NETLIB

Review Comment:
   `select` follows what the other netutils users of netlib do (`netutils/iperf`, `discover`, `netinit`). `NETUTILS_NETLIB` only depends on `NET`, which `NET_IPv4` and `NET_BRIDGE` already imply here, so selecting it is safe and a defconfig does not need to enable it by hand.



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,378 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include <nuttx/net/ioctl.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_ifcmd
+ *
+ * Description:
+ *   Add or remove a bridge port.
+ *
+ ****************************************************************************/
+
+static int wifibridge_ifcmd(int sock, int cmd, FAR const char *bridge,
+                            FAR const char *ifname)
+{
+  struct ifreq req;
+
+  memset(&req, 0, sizeof(req));
+  strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+  req.ifr_ifindex = if_nametoindex(ifname);
+  if (req.ifr_ifindex == 0)
+    {
+      return -ENODEV;

Review Comment:
   Done. `if_nametoindex()` sets errno, so the error is now propagated instead of a fixed `-ENODEV`. This code now lives in netlib (see the comment below), which returns -1 with errno, and `wifibridge` converts it to `-errno`.



##########
system/wifibridge/README.md:
##########
@@ -0,0 +1,44 @@
+# wifibridge
+
+Wi-Fi SoftAP to Ethernet bridge for NuttX.  It turns a Wi-Fi interface

Review Comment:
   Removed the README from this PR. I will send the documentation page to apache/nuttx as a separate PR under `Documentation/applications/system/wifibridge`.



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,378 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include <nuttx/net/ioctl.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_ifcmd
+ *
+ * Description:
+ *   Add or remove a bridge port.
+ *
+ ****************************************************************************/
+
+static int wifibridge_ifcmd(int sock, int cmd, FAR const char *bridge,

Review Comment:
   Yes, done. Added `netlib_add_bridge()`, `netlib_del_bridge()`, `netlib_bridge_addif()` and `netlib_bridge_delif()` in `netutils/netlib/netlib_bridge.c` (built with `CONFIG_NET_BRIDGE`). `wifibridge` now uses them instead of its private ioctl wrapper, and `system/brctl` is switched to them in a separate commit, which removes the duplicated `brctl_ifcmd()`. The PR now has three commits: netlib, wifibridge, brctl.
   
   Re-tested on an EV49N51A: five `wifibridge start`/`stop` cycles with no memory growth, `brctl addbr`/`addif`/`delif`/`delbr` including the error cases, and a Wi-Fi station getting DHCP from the board and pinging a host on the Ethernet port through the bridge.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "daniel-p-carvalho (via GitHub)" <gi...@apache.org>.
daniel-p-carvalho commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4312243721


##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE

Review Comment:
   Done. I removed the `#ifdef CONFIG_NET_BRIDGE` guard and its closing `#endif`, since both the Makefile and the CMakeLists.txt only build this file when `CONFIG_NET_BRIDGE` is set. I kept `#include <nuttx/config.h>`, which the other netlib files also include. Let me know if you meant different lines.



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;

Review Comment:
   At this point `if_nametoindex()` succeeded, meaning the bridge already exists, so `errno` is not set and `-EEXIST` is the right value. I kept the check before touching the interfaces so a second `start` does not reconfigure a running bridge. Did you mean something else 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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "daniel-p-carvalho (via GitHub)" <gi...@apache.org>.
daniel-p-carvalho commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4312180370


##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;

Review Comment:
   Done. The bridge functions in netlib now return a negated errno value as suggested, so this case is `-EINVAL`.



##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;
+    }
+
+  sockfd = socket(NET_SOCK_FAMILY, NET_SOCK_TYPE, NET_SOCK_PROTOCOL);
+  if (sockfd < 0)
+    {
+      return ERROR;
+    }
+
+  if (ifname == NULL)
+    {
+      ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)bridge);
+    }
+  else
+    {
+      memset(&req, 0, sizeof(req));
+      strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+      req.ifr_ifindex = if_nametoindex(ifname);
+      if (req.ifr_ifindex == 0)
+        {
+          ret = ERROR;
+        }
+      else
+        {
+          ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)&req);
+        }
+    }
+
+  close(sockfd);
+  return ret;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_add_bridge
+ *
+ * Description:
+ *   Create a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to create
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_add_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRADDBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_del_bridge
+ *
+ * Description:
+ *   Delete a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to delete
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_del_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRDELBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_addif
+ *
+ * Description:
+ *   Add a network device to a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to add
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_addif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :

Review Comment:
   Done, `-EINVAL` for a NULL `ifname`.



##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;
+    }
+
+  sockfd = socket(NET_SOCK_FAMILY, NET_SOCK_TYPE, NET_SOCK_PROTOCOL);
+  if (sockfd < 0)
+    {
+      return ERROR;
+    }
+
+  if (ifname == NULL)
+    {
+      ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)bridge);
+    }
+  else
+    {
+      memset(&req, 0, sizeof(req));
+      strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+      req.ifr_ifindex = if_nametoindex(ifname);
+      if (req.ifr_ifindex == 0)
+        {
+          ret = ERROR;
+        }
+      else
+        {
+          ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)&req);
+        }
+    }
+
+  close(sockfd);
+  return ret;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_add_bridge
+ *
+ * Description:
+ *   Create a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to create
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_add_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRADDBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_del_bridge
+ *
+ * Description:
+ *   Delete a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to delete
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_del_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRDELBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_addif
+ *
+ * Description:
+ *   Add a network device to a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to add
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_addif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :
+         netlib_bridge_cmd(SIOCBRADDIF, bridge, ifname);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_delif
+ *
+ * Description:
+ *   Remove a network device from a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to remove
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_delif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :

Review Comment:
   Done, same as above.



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;
+    }
+
+  sock = wapi_make_socket();
+  if (sock < 0)
+    {
+      return sock;
+    }
+
+  /* The ports carry no address, only the bridge does */
+
+  none.s_addr = INADDR_ANY;

Review Comment:
   Done, `none` is now set after `wifibridge_setup_softap()`.



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;
+    }
+
+  sock = wapi_make_socket();
+  if (sock < 0)
+    {
+      return sock;
+    }
+
+  /* The ports carry no address, only the bridge does */
+
+  none.s_addr = INADDR_ANY;
+
+  ret = wifibridge_setup_softap(sock, config);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_set_ipv4addr(config->wlanif, &none);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_set_ipv4addr(config->ethif, &none);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  /* A bridge port must be up to carry traffic */
+
+  ret = netlib_ifup(config->ethif);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_ifup(config->wlanif);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  if (netlib_add_bridge(config->bridge) < 0)
+    {
+      ret = -errno;

Review Comment:
   Done. `netlib_add_bridge()`, `netlib_del_bridge()`, `netlib_bridge_addif()` and `netlib_bridge_delif()` now return 0 or a negated errno value, and `wifibridge` and `brctl` use that value directly (`brctl` prints `strerror(-ret)`). Re-tested on the EV49N51A: `brctl` error cases and five `wifibridge start`/`stop` cycles.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4309614753


##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;
+    }
+
+  sockfd = socket(NET_SOCK_FAMILY, NET_SOCK_TYPE, NET_SOCK_PROTOCOL);
+  if (sockfd < 0)
+    {
+      return ERROR;
+    }
+
+  if (ifname == NULL)
+    {
+      ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)bridge);
+    }
+  else
+    {
+      memset(&req, 0, sizeof(req));
+      strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+      req.ifr_ifindex = if_nametoindex(ifname);
+      if (req.ifr_ifindex == 0)
+        {
+          ret = ERROR;
+        }
+      else
+        {
+          ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)&req);
+        }
+    }
+
+  close(sockfd);
+  return ret;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_add_bridge
+ *
+ * Description:
+ *   Create a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to create
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_add_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRADDBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_del_bridge
+ *
+ * Description:
+ *   Delete a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to delete
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_del_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRDELBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_addif
+ *
+ * Description:
+ *   Add a network device to a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to add
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_addif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :
+         netlib_bridge_cmd(SIOCBRADDIF, bridge, ifname);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_delif
+ *
+ * Description:
+ *   Remove a network device from a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to remove
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_delif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :

Review Comment:
   ditto



##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE

Review Comment:
   remove the above two lines



##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;
+    }
+
+  sockfd = socket(NET_SOCK_FAMILY, NET_SOCK_TYPE, NET_SOCK_PROTOCOL);
+  if (sockfd < 0)
+    {
+      return ERROR;
+    }
+
+  if (ifname == NULL)
+    {
+      ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)bridge);
+    }
+  else
+    {
+      memset(&req, 0, sizeof(req));
+      strlcpy(req.ifr_name, bridge, IFNAMSIZ);
+      req.ifr_ifindex = if_nametoindex(ifname);
+      if (req.ifr_ifindex == 0)
+        {
+          ret = ERROR;
+        }
+      else
+        {
+          ret = ioctl(sockfd, cmd, (unsigned long)(uintptr_t)&req);
+        }
+    }
+
+  close(sockfd);
+  return ret;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_add_bridge
+ *
+ * Description:
+ *   Create a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to create
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_add_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRADDBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_del_bridge
+ *
+ * Description:
+ *   Delete a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge to delete
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set
+ *
+ ****************************************************************************/
+
+int netlib_del_bridge(FAR const char *bridge)
+{
+  return netlib_bridge_cmd(SIOCBRDELBR, bridge, NULL);
+}
+
+/****************************************************************************
+ * Name: netlib_bridge_addif
+ *
+ * Description:
+ *   Add a network device to a bridge.
+ *
+ * Parameters:
+ *   bridge - The name of the bridge
+ *   ifname - The name of the network device to add
+ *
+ * Return:
+ *   0 on success; -1 on failure with errno set (the device must exist)
+ *
+ ****************************************************************************/
+
+int netlib_bridge_addif(FAR const char *bridge, FAR const char *ifname)
+{
+  return ifname == NULL ? ERROR :

Review Comment:
   need set errno



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;

Review Comment:
   -errno



##########
netutils/netlib/netlib_bridge.c:
##########
@@ -0,0 +1,177 @@
+/****************************************************************************
+ * apps/netutils/netlib/netlib_bridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#ifdef CONFIG_NET_BRIDGE
+
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+
+#include <nuttx/net/ioctl.h>
+#include <nuttx/net/netconfig.h>
+
+#include "netutils/netlib.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: netlib_bridge_cmd
+ *
+ * Description:
+ *   Issue a bridge ioctl that takes a bridge name only (add/delete bridge)
+ *   or a bridge name and an interface (add/delete port).
+ *
+ ****************************************************************************/
+
+static int netlib_bridge_cmd(int cmd, FAR const char *bridge,
+                             FAR const char *ifname)
+{
+  struct ifreq req;
+  int sockfd;
+  int ret;
+
+  if (bridge == NULL)
+    {
+      return ERROR;

Review Comment:
   errno = EINVAL



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;
+    }
+
+  sock = wapi_make_socket();
+  if (sock < 0)
+    {
+      return sock;
+    }
+
+  /* The ports carry no address, only the bridge does */
+
+  none.s_addr = INADDR_ANY;
+
+  ret = wifibridge_setup_softap(sock, config);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_set_ipv4addr(config->wlanif, &none);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_set_ipv4addr(config->ethif, &none);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  /* A bridge port must be up to carry traffic */
+
+  ret = netlib_ifup(config->ethif);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  ret = netlib_ifup(config->wlanif);
+  if (ret < 0)
+    {
+      goto errout;
+    }
+
+  if (netlib_add_bridge(config->bridge) < 0)
+    {
+      ret = -errno;

Review Comment:
   let's return -errno for the new bridge  netutils functions like netlib_ifup



##########
netutils/wifibridge/wifibridge.c:
##########
@@ -0,0 +1,351 @@
+/****************************************************************************
+ * apps/netutils/wifibridge/wifibridge.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/socket.h>
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <net/if.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+
+#include "netutils/netlib.h"
+#include "netutils/wifibridge.h"
+#include "wireless/wapi.h"
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+#  include "netutils/dhcpd.h"
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_setup_softap
+ *
+ * Description:
+ *   Put the Wi-Fi interface in master mode with the requested security,
+ *   channel and SSID.
+ *
+ ****************************************************************************/
+
+static int wifibridge_setup_softap(int sock,
+                                   FAR const struct wifibridge_config_s *cfg)
+{
+  size_t passlen = strnlen(cfg->passphrase, 64);
+  int version;
+  int ret;
+
+  if (passlen < 8 || passlen > 63 || cfg->channel < 1 || cfg->channel > 13)
+    {
+      return -EINVAL;
+    }
+
+  version = cfg->security == WIFIBRIDGE_SEC_WPA3 ?
+            IW_AUTH_WPA_VERSION_WPA3 : IW_AUTH_WPA_VERSION_WPA2;
+
+  ret = wapi_set_mode(sock, cfg->wlanif, WAPI_MODE_MASTER);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_WPA_VERSION, version);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_auth_param(sock, cfg->wlanif,
+                                       IW_AUTH_CIPHER_PAIRWISE,
+                                       IW_AUTH_CIPHER_CCMP);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wpa_driver_wext_set_key_ext(sock, cfg->wlanif, WPA_ALG_CCMP,
+                                    cfg->passphrase, passlen);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  ret = wapi_set_freq(sock, cfg->wlanif, cfg->channel, WAPI_FREQ_FIXED);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  return wapi_set_essid(sock, cfg->wlanif, cfg->ssid, WAPI_ESSID_ON);
+}
+
+/****************************************************************************
+ * Name: wifibridge_teardown
+ *
+ * Description:
+ *   Undo the steps of wifibridge_start() in reverse order.  Each step is
+ *   tried even if an earlier one failed, since this also cleans up a
+ *   partially started bridge.
+ *
+ ****************************************************************************/
+
+static int wifibridge_teardown(int sock,
+                               FAR const struct wifibridge_config_s *cfg)
+{
+  int ret = OK;
+
+#ifdef CONFIG_NETUTILS_WIFIBRIDGE_DHCPD
+  dhcpd_stop();
+#endif
+
+  netlib_ifdown(cfg->bridge);
+
+  netlib_bridge_delif(cfg->bridge, cfg->wlanif);
+  netlib_bridge_delif(cfg->bridge, cfg->ethif);
+
+  if (netlib_del_bridge(cfg->bridge) < 0)
+    {
+      ret = -errno;
+    }
+
+  /* Bringing the interface down does not stop the AP, an ESSID set with the
+   * off flag does.
+   */
+
+  wapi_set_essid(sock, cfg->wlanif, "", WAPI_ESSID_OFF);
+  wapi_set_ifdown(sock, cfg->wlanif);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: wifibridge_isup
+ ****************************************************************************/
+
+static bool wifibridge_isup(int sock, FAR const char *ifname)
+{
+  int is_up;
+
+  return wapi_get_ifup(sock, ifname, &is_up) >= 0 && is_up != 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: wifibridge_start
+ ****************************************************************************/
+
+int wifibridge_start(FAR const struct wifibridge_config_s *config)
+{
+  struct in_addr none;
+  struct in_addr addr;
+  int sock;
+  int ret;
+
+  if (config == NULL || config->bridge == NULL || config->ethif == NULL ||
+      config->wlanif == NULL || config->ssid == NULL ||
+      config->passphrase == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (if_nametoindex(config->bridge) != 0)
+    {
+      return -EEXIST;
+    }
+
+  sock = wapi_make_socket();
+  if (sock < 0)
+    {
+      return sock;
+    }
+
+  /* The ports carry no address, only the bridge does */
+
+  none.s_addr = INADDR_ANY;

Review Comment:
   move after line 204



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] system/wifibridge: add a Wi-Fi SoftAP to Ethernet bridge command. [nuttx-apps]

Posted by "daniel-p-carvalho (via GitHub)" <gi...@apache.org>.
daniel-p-carvalho commented on code in PR #3876:
URL: https://github.com/apache/nuttx-apps/pull/3876#discussion_r4308836034


##########
system/wifibridge/README.md:
##########
@@ -0,0 +1,44 @@
+# wifibridge
+
+Wi-Fi SoftAP to Ethernet bridge for NuttX.  It turns a Wi-Fi interface

Review Comment:
   The documentation page is now in apache/nuttx#20793.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org