[PATCH stable] Separate multicast configuration for mesh and wlan interfaces.
Dan Williams
dcbw at redhat.com
Mon May 19 07:01:59 EDT 2008
On Sun, 2008-05-18 at 20:48 +0100, David Woodhouse wrote:
> On Tue, 2008-05-13 at 15:38 +0100, David Woodhouse wrote:
> > > On an SMP host, are you sure we can't end up setting the multicast list
> > > simultaneously on the two logical devices?
> >
> > (A: No.)
>
> Try it like this... completely untested and hence probably broken in
> some stupid and minor way, but testing is something for tomorrow, not
> Sunday night when I'm supposed to be cooking dinner.
>
> We now merge duplicates from the two address lists while we're building
> the CMD_MAC_MULTICAST_ADR packet to send to the device, so we don't
> artificially limit each device to MRVDRV_MAX_MULTICAST_LIST_SIZE/2
> addresses. We'll go into allmulti mode only if the total number of
> addresses, excluding duplicates, exceeds the limit. Although I'm not
> wonderfully happy that we don't have any way of interrogating the
> firmware for its limit; what happens when we send more addresses than
> the firmware can cope with?
Is the firmware multicast address limit the same for every firmware from
5.0.x up to 9? Is it something that people with the firmware dev kit
can change with a recompile? Because if it changes between any of the
firmware revisions already out there (including for 8385 CF, 8686 SDIO,
8388 USB, etc) then we'll probably need a lookup table. I just hope the
different firmwares does something sensible when they get more than they
can handle?
Dan
> We also deal with the locking issues -- that we could be in
> lbs_set_multicast_list() for eth0 and msh0 simultaneously on two
> different CPUs -- by punting the actual work to a workqueue. Which can
> lock and use the multicast lists directly from each device, so we don't
> need our own copy of each.
>
> And it moves CMD_MAC_MULTICAST_ADR to a direct command, as we have been
> doing for all commands.
>
> Overall, it results in the net addition of five lines of code, instead
> of the 64 lines added by the previous version :)
> diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
> index 6328b95..2473ba8 100644
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -778,28 +778,6 @@ out:
> return ret;
> }
>
> -static int lbs_cmd_mac_multicast_adr(struct lbs_private *priv,
> - struct cmd_ds_command *cmd,
> - u16 cmd_action)
> -{
> - struct cmd_ds_mac_multicast_adr *pMCastAdr = &cmd->params.madr;
> -
> - lbs_deb_enter(LBS_DEB_CMD);
> - cmd->size = cpu_to_le16(sizeof(struct cmd_ds_mac_multicast_adr) +
> - S_DS_GEN);
> - cmd->command = cpu_to_le16(CMD_MAC_MULTICAST_ADR);
> -
> - lbs_deb_cmd("MULTICAST_ADR: setting %d addresses\n", pMCastAdr->nr_of_adrs);
> - pMCastAdr->action = cpu_to_le16(cmd_action);
> - pMCastAdr->nr_of_adrs =
> - cpu_to_le16((u16) priv->nr_of_multicastmacaddr);
> - memcpy(pMCastAdr->maclist, priv->multicastlist,
> - priv->nr_of_multicastmacaddr * ETH_ALEN);
> -
> - lbs_deb_leave(LBS_DEB_CMD);
> - return 0;
> -}
> -
> /**
> * @brief Get the radio channel
> *
> @@ -1279,8 +1257,7 @@ void lbs_set_mac_control(struct lbs_private *priv)
> cmd.action = cpu_to_le16(priv->mac_control);
> cmd.reserved = 0;
>
> - lbs_cmd_async(priv, CMD_MAC_CONTROL,
> - &cmd.hdr, sizeof(cmd));
> + lbs_cmd_async(priv, CMD_MAC_CONTROL, &cmd.hdr, sizeof(cmd));
>
> lbs_deb_leave(LBS_DEB_CMD);
> }
> @@ -1392,10 +1369,6 @@ int lbs_prepare_and_send_command(struct lbs_private *priv,
> cmdptr, cmd_action);
> break;
>
> - case CMD_MAC_MULTICAST_ADR:
> - ret = lbs_cmd_mac_multicast_adr(priv, cmdptr, cmd_action);
> - break;
> -
> case CMD_802_11_MONITOR_MODE:
> ret = lbs_cmd_802_11_monitor_mode(cmdptr,
> cmd_action, pdata_buf);
> @@ -1484,7 +1457,7 @@ int lbs_prepare_and_send_command(struct lbs_private *priv,
> ret = lbs_cmd_bcn_ctrl(priv, cmdptr, cmd_action);
> break;
> default:
> - lbs_deb_host("PREP_CMD: unknown command 0x%04x\n", cmd_no);
> + lbs_pr_err("PREP_CMD: unknown command 0x%04x\n", cmd_no);
> ret = -1;
> break;
> }
> diff --git a/drivers/net/wireless/libertas/cmdresp.c b/drivers/net/wireless/libertas/cmdresp.c
> index 5abecb7..4c3c5ec 100644
> --- a/drivers/net/wireless/libertas/cmdresp.c
> +++ b/drivers/net/wireless/libertas/cmdresp.c
> @@ -316,7 +316,6 @@ static inline int handle_cmd_response(struct lbs_private *priv,
>
> break;
>
> - case CMD_RET(CMD_MAC_MULTICAST_ADR):
> case CMD_RET(CMD_802_11_RESET):
> case CMD_RET(CMD_802_11_AUTHENTICATE):
> case CMD_RET(CMD_802_11_BEACON_STOP):
> @@ -376,8 +375,8 @@ static inline int handle_cmd_response(struct lbs_private *priv,
> break;
>
> default:
> - lbs_deb_host("CMD_RESP: unknown cmd response 0x%04x\n",
> - le16_to_cpu(resp->command));
> + lbs_pr_err("CMD_RESP: unknown cmd response 0x%04x\n",
> + le16_to_cpu(resp->command));
> break;
> }
> lbs_deb_leave(LBS_DEB_HOST);
> diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
> index 0d9edb9..e12ce65 100644
> --- a/drivers/net/wireless/libertas/dev.h
> +++ b/drivers/net/wireless/libertas/dev.h
> @@ -140,6 +140,8 @@ struct lbs_private {
> wait_queue_head_t waitq;
> struct workqueue_struct *work_thread;
>
> + struct work_struct mcast_work;
> +
> /** Scanning */
> struct delayed_work scan_work;
> struct delayed_work assoc_work;
> diff --git a/drivers/net/wireless/libertas/hostcmd.h b/drivers/net/wireless/libertas/hostcmd.h
> index f29bc5b..c36ab31 100644
> --- a/drivers/net/wireless/libertas/hostcmd.h
> +++ b/drivers/net/wireless/libertas/hostcmd.h
> @@ -219,6 +219,7 @@ struct cmd_ds_mac_control {
> };
>
> struct cmd_ds_mac_multicast_adr {
> + struct cmd_header hdr;
> __le16 action;
> __le16 nr_of_adrs;
> u8 maclist[ETH_ALEN * MRVDRV_MAX_MULTICAST_LIST_SIZE];
> @@ -703,7 +704,6 @@ struct cmd_ds_command {
> struct cmd_ds_802_11_rf_antenna rant;
> struct cmd_ds_802_11_monitor_mode monitor;
> struct cmd_ds_802_11_rate_adapt_rateset rateset;
> - struct cmd_ds_mac_multicast_adr madr;
> struct cmd_ds_802_11_ad_hoc_join adj;
> struct cmd_ds_802_11_rssi rssi;
> struct cmd_ds_802_11_rssi_rsp rssirsp;
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index 406f54d..b0c7fbd 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -11,6 +11,7 @@
> #include <linux/if_arp.h>
> #include <linux/kthread.h>
> #include <linux/kfifo.h>
> +#include <linux/stddef.h>
>
> #include <net/iw_handler.h>
> #include <net/ieee80211.h>
> @@ -563,89 +564,116 @@ done:
> return ret;
> }
>
> -static int lbs_copy_multicast_address(struct lbs_private *priv,
> - struct net_device *dev)
> +static void lbs_set_mcast_worker(struct work_struct *work)
> {
> - int i = 0;
> - struct dev_mc_list *mcptr = dev->mc_list;
> -
> - for (i = 0; i < dev->mc_count; i++) {
> - memcpy(&priv->multicastlist[i], mcptr->dmi_addr, ETH_ALEN);
> - mcptr = mcptr->next;
> - }
> - return i;
> -}
> -
> -static void lbs_set_multicast_list(struct net_device *dev)
> -{
> - struct lbs_private *priv = dev->priv;
> - int old_mac_control;
> + struct lbs_private *priv = container_of(work, struct lbs_private, mcast_work);
> + struct cmd_ds_mac_multicast_adr mcast_cmd;
> + struct dev_mc_list *mc_list;
> DECLARE_MAC_BUF(mac);
> + int dev_flags;
> + int i, j, nr_eth_addrs;
> + int old_mac_control = priv->mac_control;
>
> - lbs_deb_enter(LBS_DEB_NET);
> -
> - old_mac_control = priv->mac_control;
> -
> - if (dev->flags & IFF_PROMISC) {
> - lbs_deb_net("enable promiscuous mode\n");
> - priv->mac_control |=
> - CMD_ACT_MAC_PROMISCUOUS_ENABLE;
> - priv->mac_control &=
> - ~(CMD_ACT_MAC_ALL_MULTICAST_ENABLE |
> - CMD_ACT_MAC_MULTICAST_ENABLE);
> - } else {
> - /* Multicast */
> - priv->mac_control &=
> - ~CMD_ACT_MAC_PROMISCUOUS_ENABLE;
> -
> - if (dev->flags & IFF_ALLMULTI || dev->mc_count >
> - MRVDRV_MAX_MULTICAST_LIST_SIZE) {
> - lbs_deb_net( "enabling all multicast\n");
> - priv->mac_control |=
> - CMD_ACT_MAC_ALL_MULTICAST_ENABLE;
> - priv->mac_control &=
> - ~CMD_ACT_MAC_MULTICAST_ENABLE;
> - } else {
> - priv->mac_control &=
> - ~CMD_ACT_MAC_ALL_MULTICAST_ENABLE;
> -
> - if (!dev->mc_count) {
> - lbs_deb_net("no multicast addresses, "
> - "disabling multicast\n");
> - priv->mac_control &=
> - ~CMD_ACT_MAC_MULTICAST_ENABLE;
> - } else {
> - int i;
> -
> - priv->mac_control |=
> - CMD_ACT_MAC_MULTICAST_ENABLE;
> + lbs_deb_enter(LBS_DEB_MAIN);
>
> - priv->nr_of_multicastmacaddr =
> - lbs_copy_multicast_address(priv, dev);
> + dev_flags = priv->dev->flags;
> + if (priv->mesh_dev)
> + dev_flags |= priv->mesh_dev->flags;
> +
> + if (dev_flags & IFF_PROMISC) {
> + priv->mac_control |= CMD_ACT_MAC_PROMISCUOUS_ENABLE;
> + priv->mac_control &= ~(CMD_ACT_MAC_ALL_MULTICAST_ENABLE |
> + CMD_ACT_MAC_MULTICAST_ENABLE);
> + goto out_set_mac_control;
> + } else if (dev_flags & IFF_PROMISC) {
> + do_allmulti:
> + priv->mac_control |= CMD_ACT_MAC_ALL_MULTICAST_ENABLE;
> + priv->mac_control &= ~(CMD_ACT_MAC_PROMISCUOUS_ENABLE |
> + CMD_ACT_MAC_MULTICAST_ENABLE);
> + goto out_set_mac_control;
> + }
>
> - lbs_deb_net("multicast addresses: %d\n",
> - dev->mc_count);
> + mcast_cmd.hdr.size = cpu_to_le16(sizeof(mcast_cmd));
> + mcast_cmd.action = cpu_to_le16(CMD_ACT_SET);
>
> - for (i = 0; i < dev->mc_count; i++) {
> - lbs_deb_net("Multicast address %d: %s\n",
> - i, print_mac(mac,
> - priv->multicastlist[i]));
> + i = 0;
> + netif_tx_lock_bh(priv->dev);
> + mc_list = priv->dev->mc_list;
> + while (mc_list) {
> + if (i == MRVDRV_MAX_MULTICAST_LIST_SIZE)
> + break;
> + memcpy(&mcast_cmd.maclist[6*i], mc_list->dmi_addr, ETH_ALEN);
> + lbs_deb_net("mcast address %s:%s added to filter\n",
> + priv->dev->name,
> + print_mac(mac, mc_list->dmi_addr));
> + i++;
> + mc_list = mc_list->next;
> + }
> + netif_tx_unlock_bh(priv->dev);
> +
> + nr_eth_addrs = i;
> +
> + if (mc_list)
> + goto do_allmulti;
> +
> + if (priv->mesh_dev) {
> + netif_tx_lock_bh(priv->mesh_dev);
> + mc_list = priv->dev->mc_list;
> + while (mc_list) {
> + for (j=0; j<nr_eth_addrs; j++) {
> + if (!memcmp(&mcast_cmd.maclist[6*j],
> + mc_list->dmi_addr, ETH_ALEN)) {
> + lbs_deb_net("mcast address %s:%s skipped (duplicate)\n",
> + priv->mesh_dev->name,
> + print_mac(mac, mc_list->dmi_addr));
> + goto next;
> }
> - /* send multicast addresses to firmware */
> - lbs_prepare_and_send_command(priv,
> - CMD_MAC_MULTICAST_ADR,
> - CMD_ACT_SET, 0, 0,
> - NULL);
> }
> + if (i == MRVDRV_MAX_MULTICAST_LIST_SIZE)
> + break;
> + memcpy(&mcast_cmd.maclist[6*i], mc_list->dmi_addr, ETH_ALEN);
> + lbs_deb_net("mcast address %s:%s added to filter\n",
> + priv->mesh_dev->name,
> + print_mac(mac, mc_list->dmi_addr));
> + i++;
> + next:
> + mc_list = mc_list->next;
> }
> +
> + netif_tx_unlock_bh(priv->mesh_dev);
> + if (mc_list)
> + goto do_allmulti;
> }
> +
> + if (i) {
> + int size = offsetof(struct cmd_ds_mac_multicast_adr, maclist[i]);
> +
> + mcast_cmd.hdr.size = cpu_to_le16(size);
> + mcast_cmd.nr_of_adrs = cpu_to_le16(i);
> +
> + lbs_cmd_async(priv, CMD_MAC_MULTICAST_ADR, &mcast_cmd.hdr, size);
> +
> + priv->mac_control |= CMD_ACT_MAC_MULTICAST_ENABLE;
> + } else
> + priv->mac_control &= CMD_ACT_MAC_MULTICAST_ENABLE;
>
> + priv->mac_control &= ~(CMD_ACT_MAC_PROMISCUOUS_ENABLE |
> + CMD_ACT_MAC_ALL_MULTICAST_ENABLE);
> +
> + out_set_mac_control:
> if (priv->mac_control != old_mac_control)
> lbs_set_mac_control(priv);
>
> lbs_deb_leave(LBS_DEB_NET);
> }
>
> +static void lbs_set_multicast_list(struct net_device *dev)
> +{
> + struct lbs_private *priv = dev->priv;
> +
> + schedule_work(&priv->mcast_work);
> +}
> +
> /**
> * @brief This function handles the major jobs in the LBS driver.
> * It handles all events generated by firmware, RX data received
> @@ -1133,6 +1161,7 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev)
> priv->work_thread = create_singlethread_workqueue("lbs_worker");
> INIT_DELAYED_WORK(&priv->assoc_work, lbs_association_worker);
> INIT_DELAYED_WORK(&priv->scan_work, lbs_scan_worker);
> + INIT_WORK(&priv->mcast_work, lbs_set_mcast_worker);
> INIT_WORK(&priv->sync_channel, lbs_sync_channel_worker);
>
> sprintf(priv->mesh_ssid, "mesh");
> @@ -1169,6 +1198,7 @@ int lbs_remove_card(struct lbs_private *priv)
>
> cancel_delayed_work(&priv->scan_work);
> cancel_delayed_work(&priv->assoc_work);
> + cancel_work_sync(&priv->mcast_work);
> destroy_workqueue(priv->work_thread);
>
> if (priv->psmode == LBS802_11POWERMODEMAX_PSP) {
> @@ -1331,6 +1361,8 @@ static int lbs_add_mesh(struct lbs_private *priv)
> #ifdef WIRELESS_EXT
> mesh_dev->wireless_handlers = (struct iw_handler_def *)&mesh_handler_def;
> #endif
> + mesh_dev->flags |= IFF_BROADCAST | IFF_MULTICAST;
> + mesh_dev->set_multicast_list = lbs_set_multicast_list;
> /* Register virtual mesh interface */
> ret = register_netdev(mesh_dev);
> if (ret) {
> @@ -1562,7 +1594,6 @@ static int lbs_add_rtap(struct lbs_private *priv)
> rtap_dev->stop = lbs_rtap_stop;
> rtap_dev->get_stats = lbs_rtap_get_stats;
> rtap_dev->hard_start_xmit = lbs_rtap_hard_start_xmit;
> - rtap_dev->set_multicast_list = lbs_set_multicast_list;
> rtap_dev->priv = priv;
>
> ret = register_netdev(rtap_dev);
>
More information about the libertas-dev
mailing list