* [PATCH bluetooth-next 0/2] mac802154: iface: better concurrent iface handling
@ 2014-12-19 22:28 Alexander Aring
2014-12-19 22:28 ` [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces Alexander Aring
2014-12-19 22:28 ` [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check Alexander Aring
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Aring @ 2014-12-19 22:28 UTC (permalink / raw)
To: linux-wpan; +Cc: kernel, Alexander Aring
This patch series contains changes for a better multiple iface handling.
We have only one phy with several mac parameters like csma, aret, etc.
Currently it could be that one interface can overwrite these parameters
while ifup when another interface is currently running. To avoid that
we compare each parameter and check if differs, if not we allow to ifup
the interface.
Alexander Aring (2):
mac802154: iface: check concurrent ifaces
ieee802154: iface: move multiple node type check
net/mac802154/iface.c | 101 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 87 insertions(+), 14 deletions(-)
--
2.2.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces
2014-12-19 22:28 [PATCH bluetooth-next 0/2] mac802154: iface: better concurrent iface handling Alexander Aring
@ 2014-12-19 22:28 ` Alexander Aring
2014-12-19 22:31 ` Marcel Holtmann
2014-12-19 22:28 ` [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check Alexander Aring
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Aring @ 2014-12-19 22:28 UTC (permalink / raw)
To: linux-wpan; +Cc: kernel, Alexander Aring
This patch adds a check for concurrent interfaces while calling
interface up. This avoids to have different mac parameters on one phy.
Otherwise it could be that a interface can overwrite current phy mac
settings which is set by an another interface.
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
net/mac802154/iface.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 9ae8930..509621a 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -175,6 +175,79 @@ err:
return res;
}
+static int
+ieee802154_check_mac_settings(struct ieee802154_local *local,
+ struct wpan_dev *wpan_dev,
+ struct wpan_dev *nwpan_dev)
+{
+ ASSERT_RTNL();
+
+ if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
+ if (wpan_dev->promiscuous_mode != nwpan_dev->promiscuous_mode)
+ return -EBUSY;
+ }
+
+ if (local->hw.flags & IEEE802154_HW_AFILT) {
+ if (wpan_dev->pan_id != nwpan_dev->pan_id)
+ return -EBUSY;
+
+ if (wpan_dev->short_addr != nwpan_dev->short_addr)
+ return -EBUSY;
+
+ if (wpan_dev->extended_addr != nwpan_dev->extended_addr)
+ return -EBUSY;
+ }
+
+ if (local->hw.flags & IEEE802154_HW_CSMA_PARAMS) {
+ if (wpan_dev->min_be != nwpan_dev->min_be)
+ return -EBUSY;
+
+ if (wpan_dev->max_be != nwpan_dev->max_be)
+ return -EBUSY;
+
+ if (wpan_dev->csma_retries != nwpan_dev->csma_retries)
+ return -EBUSY;
+ }
+
+ if (local->hw.flags & IEEE802154_HW_FRAME_RETRIES) {
+ if (wpan_dev->frame_retries != nwpan_dev->frame_retries)
+ return -EBUSY;
+ }
+
+ if (local->hw.flags & IEEE802154_HW_LBT) {
+ if (wpan_dev->lbt != nwpan_dev->lbt)
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int
+ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
+ enum nl802154_iftype iftype)
+{
+ struct ieee802154_local *local = sdata->local;
+ struct wpan_dev *wpan_dev = &sdata->wpan_dev;
+ struct ieee802154_sub_if_data *nsdata;
+
+ /* we hold the RTNL here so can safely walk the list */
+ list_for_each_entry(nsdata, &local->interfaces, list) {
+ if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
+ int ret;
+ /* check all phy mac sublayer settings are the same.
+ * We have only one phy, different values makes trouble.
+ */
+
+ ret = ieee802154_check_mac_settings(local, wpan_dev,
+ &nsdata->wpan_dev);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int mac802154_wpan_open(struct net_device *dev)
{
int rc;
@@ -183,6 +256,10 @@ static int mac802154_wpan_open(struct net_device *dev)
struct wpan_dev *wpan_dev = &sdata->wpan_dev;
struct wpan_phy *phy = sdata->local->phy;
+ rc = ieee802154_check_concurrent_iface(sdata, sdata->vif.type);
+ if (rc < 0)
+ return rc;
+
rc = mac802154_slave_open(dev);
if (rc < 0)
return rc;
--
2.2.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check
2014-12-19 22:28 [PATCH bluetooth-next 0/2] mac802154: iface: better concurrent iface handling Alexander Aring
2014-12-19 22:28 ` [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces Alexander Aring
@ 2014-12-19 22:28 ` Alexander Aring
2014-12-19 22:33 ` Marcel Holtmann
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Aring @ 2014-12-19 22:28 UTC (permalink / raw)
To: linux-wpan; +Cc: kernel, Alexander Aring
This patch moves the handling for checking on multiple node type
interface to the corresponding concurrent iface check function.
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
net/mac802154/iface.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 509621a..b5aab5a 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -137,25 +137,11 @@ static int mac802154_wpan_mac_addr(struct net_device *dev, void *p)
static int mac802154_slave_open(struct net_device *dev)
{
struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
- struct ieee802154_sub_if_data *subif;
struct ieee802154_local *local = sdata->local;
int res = 0;
ASSERT_RTNL();
- if (sdata->vif.type == NL802154_IFTYPE_NODE) {
- mutex_lock(&sdata->local->iflist_mtx);
- list_for_each_entry(subif, &sdata->local->interfaces, list) {
- if (subif != sdata &&
- subif->vif.type == sdata->vif.type &&
- ieee802154_sdata_running(subif)) {
- mutex_unlock(&sdata->local->iflist_mtx);
- return -EBUSY;
- }
- }
- mutex_unlock(&sdata->local->iflist_mtx);
- }
-
set_bit(SDATA_STATE_RUNNING, &sdata->state);
if (!local->open_count) {
@@ -234,6 +220,16 @@ ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
list_for_each_entry(nsdata, &local->interfaces, list) {
if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
int ret;
+
+ /* TODO currently we don't support multiple node types
+ * we need to run skb_clone at rx path. Check if there
+ * exist really an use case if we need to support
+ * multiple node types at the same time.
+ */
+ if (sdata->vif.type == NL802154_IFTYPE_NODE &&
+ nsdata->vif.type == NL802154_IFTYPE_NODE)
+ return -EBUSY;
+
/* check all phy mac sublayer settings are the same.
* We have only one phy, different values makes trouble.
*/
--
2.2.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces
2014-12-19 22:28 ` [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces Alexander Aring
@ 2014-12-19 22:31 ` Marcel Holtmann
0 siblings, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2014-12-19 22:31 UTC (permalink / raw)
To: Alexander Aring; +Cc: linux-wpan, kernel
Hi Alex,
> This patch adds a check for concurrent interfaces while calling
> interface up. This avoids to have different mac parameters on one phy.
> Otherwise it could be that a interface can overwrite current phy mac
> settings which is set by an another interface.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> net/mac802154/iface.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 9ae8930..509621a 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -175,6 +175,79 @@ err:
> return res;
> }
>
> +static int
> +ieee802154_check_mac_settings(struct ieee802154_local *local,
> + struct wpan_dev *wpan_dev,
> + struct wpan_dev *nwpan_dev)
> +{
> + ASSERT_RTNL();
> +
> + if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) {
> + if (wpan_dev->promiscuous_mode != nwpan_dev->promiscuous_mode)
> + return -EBUSY;
> + }
> +
> + if (local->hw.flags & IEEE802154_HW_AFILT) {
> + if (wpan_dev->pan_id != nwpan_dev->pan_id)
> + return -EBUSY;
> +
> + if (wpan_dev->short_addr != nwpan_dev->short_addr)
> + return -EBUSY;
> +
> + if (wpan_dev->extended_addr != nwpan_dev->extended_addr)
> + return -EBUSY;
> + }
> +
> + if (local->hw.flags & IEEE802154_HW_CSMA_PARAMS) {
> + if (wpan_dev->min_be != nwpan_dev->min_be)
> + return -EBUSY;
> +
> + if (wpan_dev->max_be != nwpan_dev->max_be)
> + return -EBUSY;
> +
> + if (wpan_dev->csma_retries != nwpan_dev->csma_retries)
> + return -EBUSY;
> + }
> +
> + if (local->hw.flags & IEEE802154_HW_FRAME_RETRIES) {
> + if (wpan_dev->frame_retries != nwpan_dev->frame_retries)
> + return -EBUSY;
> + }
> +
> + if (local->hw.flags & IEEE802154_HW_LBT) {
> + if (wpan_dev->lbt != nwpan_dev->lbt)
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
> + enum nl802154_iftype iftype)
> +{
> + struct ieee802154_local *local = sdata->local;
> + struct wpan_dev *wpan_dev = &sdata->wpan_dev;
> + struct ieee802154_sub_if_data *nsdata;
> +
> + /* we hold the RTNL here so can safely walk the list */
> + list_for_each_entry(nsdata, &local->interfaces, list) {
> + if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
> + int ret;
> + /* check all phy mac sublayer settings are the same.
> + * We have only one phy, different values makes trouble.
> + */
> +
I think this empty line was meant to be above the comment. So that the variable declaration is separated from the code.
> + ret = ieee802154_check_mac_settings(local, wpan_dev,
> + &nsdata->wpan_dev);
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int mac802154_wpan_open(struct net_device *dev)
> {
> int rc;
> @@ -183,6 +256,10 @@ static int mac802154_wpan_open(struct net_device *dev)
> struct wpan_dev *wpan_dev = &sdata->wpan_dev;
> struct wpan_phy *phy = sdata->local->phy;
>
> + rc = ieee802154_check_concurrent_iface(sdata, sdata->vif.type);
> + if (rc < 0)
> + return rc;
> +
> rc = mac802154_slave_open(dev);
> if (rc < 0)
> return rc;
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check
2014-12-19 22:28 ` [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check Alexander Aring
@ 2014-12-19 22:33 ` Marcel Holtmann
2014-12-19 22:36 ` Alexander Aring
0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2014-12-19 22:33 UTC (permalink / raw)
To: Alexander Aring; +Cc: linux-wpan, kernel
Hi Alex,
> This patch moves the handling for checking on multiple node type
> interface to the corresponding concurrent iface check function.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> net/mac802154/iface.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 509621a..b5aab5a 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -137,25 +137,11 @@ static int mac802154_wpan_mac_addr(struct net_device *dev, void *p)
> static int mac802154_slave_open(struct net_device *dev)
> {
> struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> - struct ieee802154_sub_if_data *subif;
> struct ieee802154_local *local = sdata->local;
> int res = 0;
>
> ASSERT_RTNL();
>
> - if (sdata->vif.type == NL802154_IFTYPE_NODE) {
> - mutex_lock(&sdata->local->iflist_mtx);
> - list_for_each_entry(subif, &sdata->local->interfaces, list) {
> - if (subif != sdata &&
> - subif->vif.type == sdata->vif.type &&
> - ieee802154_sdata_running(subif)) {
> - mutex_unlock(&sdata->local->iflist_mtx);
> - return -EBUSY;
> - }
> - }
> - mutex_unlock(&sdata->local->iflist_mtx);
> - }
> -
> set_bit(SDATA_STATE_RUNNING, &sdata->state);
>
> if (!local->open_count) {
> @@ -234,6 +220,16 @@ ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
> list_for_each_entry(nsdata, &local->interfaces, list) {
> if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
> int ret;
> +
> + /* TODO currently we don't support multiple node types
> + * we need to run skb_clone at rx path. Check if there
> + * exist really an use case if we need to support
> + * multiple node types at the same time.
> + */
> + if (sdata->vif.type == NL802154_IFTYPE_NODE &&
> + nsdata->vif.type == NL802154_IFTYPE_NODE)
> + return -EBUSY;
> +
here you end up doing it correctly and fixing it, ..
> /* check all phy mac sublayer settings are the same.
> * We have only one phy, different values makes trouble.
> */
.. but that still leaves the extra empty line after this comment. You need to redo these two patches.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check
2014-12-19 22:33 ` Marcel Holtmann
@ 2014-12-19 22:36 ` Alexander Aring
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Aring @ 2014-12-19 22:36 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-wpan, kernel
Hi Marcel,
On Fri, Dec 19, 2014 at 11:33:11PM +0100, Marcel Holtmann wrote:
> Hi Alex,
>
> > This patch moves the handling for checking on multiple node type
> > interface to the corresponding concurrent iface check function.
> >
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> > net/mac802154/iface.c | 24 ++++++++++--------------
> > 1 file changed, 10 insertions(+), 14 deletions(-)
> >
> > diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> > index 509621a..b5aab5a 100644
> > --- a/net/mac802154/iface.c
> > +++ b/net/mac802154/iface.c
> > @@ -137,25 +137,11 @@ static int mac802154_wpan_mac_addr(struct net_device *dev, void *p)
> > static int mac802154_slave_open(struct net_device *dev)
> > {
> > struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
> > - struct ieee802154_sub_if_data *subif;
> > struct ieee802154_local *local = sdata->local;
> > int res = 0;
> >
> > ASSERT_RTNL();
> >
> > - if (sdata->vif.type == NL802154_IFTYPE_NODE) {
> > - mutex_lock(&sdata->local->iflist_mtx);
> > - list_for_each_entry(subif, &sdata->local->interfaces, list) {
> > - if (subif != sdata &&
> > - subif->vif.type == sdata->vif.type &&
> > - ieee802154_sdata_running(subif)) {
> > - mutex_unlock(&sdata->local->iflist_mtx);
> > - return -EBUSY;
> > - }
> > - }
> > - mutex_unlock(&sdata->local->iflist_mtx);
> > - }
> > -
> > set_bit(SDATA_STATE_RUNNING, &sdata->state);
> >
> > if (!local->open_count) {
> > @@ -234,6 +220,16 @@ ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
> > list_for_each_entry(nsdata, &local->interfaces, list) {
> > if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
> > int ret;
> > +
> > + /* TODO currently we don't support multiple node types
> > + * we need to run skb_clone at rx path. Check if there
> > + * exist really an use case if we need to support
> > + * multiple node types at the same time.
> > + */
> > + if (sdata->vif.type == NL802154_IFTYPE_NODE &&
> > + nsdata->vif.type == NL802154_IFTYPE_NODE)
> > + return -EBUSY;
> > +
>
> here you end up doing it correctly and fixing it, ..
>
> > /* check all phy mac sublayer settings are the same.
> > * We have only one phy, different values makes trouble.
> > */
>
> .. but that still leaves the extra empty line after this comment. You need to redo these two patches.
>
ok.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-19 22:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-19 22:28 [PATCH bluetooth-next 0/2] mac802154: iface: better concurrent iface handling Alexander Aring
2014-12-19 22:28 ` [PATCH bluetooth-next 1/2] mac802154: iface: check concurrent ifaces Alexander Aring
2014-12-19 22:31 ` Marcel Holtmann
2014-12-19 22:28 ` [PATCH bluetooth-next 2/2] ieee802154: iface: move multiple node type check Alexander Aring
2014-12-19 22:33 ` Marcel Holtmann
2014-12-19 22:36 ` Alexander Aring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.