diff mbox

Fix for interface names parameter ignores subsequent parameters

Message ID CALUxAYmYG7QxaeB=3o_xie3JGD2yB6ed0b1dTq1iD=0LGcvQSw@mail.gmail.com
State Rejected
Headers show

Commit Message

stannous@cumulusnetworks.com Nov. 7, 2016, 6:43 p.m. UTC
When using -i to specify one or more comma separated interfaces
(that all use the same config file), we also need to change
the loop parameters (interface.count) so that all the interfaces get
initialized.  Also, we need to make sure that all the
hostapd_interface_init()
calls are made with the same config filename.

This patch fixes hostapd/main.c to correct the interface count
and use of the one config file in the interface initialization call.

Thanks,
Sam Tannous
Cumulus Networks

Signed-off-by: Sam Tannous <stannous@cumulusnetworks.com>

--

Comments

Jouni Malinen Nov. 19, 2016, 8:26 p.m. UTC | #1
On Mon, Nov 07, 2016 at 01:43:36PM -0500, Sam Tannous wrote:
> When using -i to specify one or more comma separated interfaces
> (that all use the same config file), we also need to change
> the loop parameters (interface.count) so that all the interfaces get
> initialized.  Also, we need to make sure that all the
> hostapd_interface_init()
> calls are made with the same config filename.
> 
> This patch fixes hostapd/main.c to correct the interface count
> and use of the one config file in the interface initialization call.

I don't think this is the way this argument was supposed to work. In
other words, it is more flexible and allows some interfaces to use
different configuration files as needed. In any case, there would need
to be a configuration file listed for each interface on the command line
even if they were to be the same file for more than one interface. For
example:

hostapd -i wlan0,wlan1,wlan2 a.conf b.conf a.conf

This would use a.conf configuration file for both wlan0 and wlan2 while
b.conf is used for wlan1.
stannous@cumulusnetworks.com Nov. 19, 2016, 8:40 p.m. UTC | #2
Hi Jouni,

Thanks for the clarification.  I did misunderstand the -i option
(wishful thinking, I guess ;-)).

For our purposes, we have lots of wired interfaces  (under 60 wired interfaces)
all using the same config file with the same radius parameters.  So my
temporary patch (just starting hostapd with -i foo1,foo2,foo3...
radiusForAll.conf) seems to be working.

Ideally, I would like to just have *one* config file without the -i option
and just specify a new "interfaces" (note the plural) option
("interfaces=swp1,swp2,...swpN") as the first line.   Our CLI
can create this file and then restart/reload hostapd as needed.

(The other features we plan to add are adding netlink awareness to the
wired driver and
adding an removing (layer2) filtering based on netlink messages and
authentication
status of particular stations.  I've seen some filtering patches where
this has been tried
in the past. The plan is to use the linux tc filtering rules called
from hostapd.

Thanks,
Sam Tannous
Cumulus Networks


On Sat, Nov 19, 2016 at 3:26 PM, Jouni Malinen <j@w1.fi> wrote:
> On Mon, Nov 07, 2016 at 01:43:36PM -0500, Sam Tannous wrote:
>> When using -i to specify one or more comma separated interfaces
>> (that all use the same config file), we also need to change
>> the loop parameters (interface.count) so that all the interfaces get
>> initialized.  Also, we need to make sure that all the
>> hostapd_interface_init()
>> calls are made with the same config filename.
>>
>> This patch fixes hostapd/main.c to correct the interface count
>> and use of the one config file in the interface initialization call.
>
> I don't think this is the way this argument was supposed to work. In
> other words, it is more flexible and allows some interfaces to use
> different configuration files as needed. In any case, there would need
> to be a configuration file listed for each interface on the command line
> even if they were to be the same file for more than one interface. For
> example:
>
> hostapd -i wlan0,wlan1,wlan2 a.conf b.conf a.conf
>
> This would use a.conf configuration file for both wlan0 and wlan2 while
> b.conf is used for wlan1.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
Jouni Malinen Nov. 19, 2016, 9:49 p.m. UTC | #3
On Sat, Nov 19, 2016 at 03:40:35PM -0500, Sam Tannous wrote:
> Ideally, I would like to just have *one* config file without the -i option
> and just specify a new "interfaces" (note the plural) option
> ("interfaces=swp1,swp2,...swpN") as the first line.   Our CLI
> can create this file and then restart/reload hostapd as needed.

Do you need to even use a configuration file? hostapd control interface
can be used to dynamically add and remove interfaces, set configuration
parameters for those interfaces, and enable/disable interfaces. That's
what I would be using if there is any need for dynamic operations
instead of trying to stop and restart the process.

> (The other features we plan to add are adding netlink awareness to the
> wired driver and
> adding an removing (layer2) filtering based on netlink messages and
> authentication
> status of particular stations.  I've seen some filtering patches where
> this has been tried
> in the past. The plan is to use the linux tc filtering rules called
> from hostapd.

It would be interesting to see if something could be added to the
upstream version to get more functionality in this area.. I'm pretty
sure there are multiple implementations doing something like this, but
not very promising contributions to add that into hostap.git yet.
stannous@cumulusnetworks.com Nov. 19, 2016, 11:16 p.m. UTC | #4
Hi Jouni,

I don't necessarily need a config file if hostapd_cli can do the following:
enable/disable interface (for wired) and modify radius attributes.
From what I could find, it seems to only be used to show various mibs,
collect station information, and so on.
Perhaps I need to modify some configs and recompile?

How would I do the following?....

1. Enable driver wired for say eth1,  also disable EAP wired listening on eth3
2. Set a radius IP auth address 10.10.10.10, same for accounting,
set auth and acct ports.
3. Monitor a port for authentication (I think this is easy enough to just
detect when EAP completes with success or failure).
4. Detect link up/down so I can add/remove filtering on ports.  I could probably
do this outside hostapd but it would be nice to be able to control
this via hostapd_cli.

Thanks,
Sam

On Sat, Nov 19, 2016 at 4:49 PM, Jouni Malinen <j@w1.fi> wrote:
> Do you need to even use a configuration file? hostapd control interface
> can be used to dynamically add and remove interfaces, set configuration
> parameters for those interfaces, and enable/disable interfaces. That's
> what I would be using if there is any need for dynamic operations
> instead of trying to stop and restart the process.
>
diff mbox

Patch

diff --git a/hostapd/main.c b/hostapd/main.c
index 2c8dbd3..69fdd26 100644
--- a/hostapd/main.c
+++ b/hostapd/main.c
@@ -644,6 +644,7 @@  int main(int argc, char *argv[])
  int start_ifaces_in_sync = 0;
  char **if_names = NULL;
  size_t if_names_size = 0;
+ const char *config_fname = NULL;

  if (os_program_init())
  return -1;
@@ -756,7 +757,17 @@  int main(int argc, char *argv[])
  }
 #endif /* CONFIG_DEBUG_LINUX_TRACING */

- interfaces.count = argc - optind;
+ /*
+ * We may have comma separated multiple interfaces configured with -i.
+ * If this is the case, we must only have one config file.
+ */
+ if (if_names && if_names_size) {
+ interfaces.count = if_names_size;
+ /* the last arg must be the one config file */
+ config_fname = argv[optind];
+ } else
+ interfaces.count = argc - optind;
+
  if (interfaces.count || num_bss_configs) {
  interfaces.iface = os_calloc(interfaces.count + num_bss_configs,
      sizeof(struct hostapd_iface *));
@@ -792,7 +803,14 @@  int main(int argc, char *argv[])
  if (i < if_names_size)
  if_name = if_names[i];

- interfaces.iface[i] = hostapd_interface_init(&interfaces,
+ /* for interface names parameter, we have one config file */
+ if (config_fname)
+ interfaces.iface[i] = hostapd_interface_init(&interfaces,
+     if_name,
+     config_fname,
+     debug);
+ else
+ interfaces.iface[i] = hostapd_interface_init(&interfaces,
      if_name,
      argv[optind + i],
      debug);