Validate the given MAC address directly in dev_set_mac_address()
if a .ndo_validate_addr function is available before calling
the .ndo_set_mac_address function.
Changed .ndo_validate_addr to take a second parameter containing
a sockaddr struct to be checked instead of the net_device dev_addr.
The behaviour of .ndo_validate_addr is now: if the second parameter
is NULL the net_device->dev_addr gets validate, if != NULL
the given parameter/sockaddr gets validated instead.
This patch series include adaptations for some drivers which
use .ndo_set_mac_address functions directly - to prevent double
checks and to enable validations via .ndo_validate_addr.
If these patches get accepted, an other series will follow
to cleanup the validation checks in the .ndo_set_mac_address
functions.
Note: resend for ML's
Danny Kukawka (8):
net: validate MAC address directly in dev_set_mac_address()
bnx2x: adopt bnx2x_validate_addr() to .ndo_validate_addr changes
cris/eth_v10: use dev_set_mac_address() instead of
e100_set_mac_address()
bcm63xx_enet: use dev_set_mac_address() instead of
bcm_enet_set_mac_address()
ethoc: add .ndo_validate_addr to net_device_ops
lantiq_etop: use dev_set_mac_address() instead of
ltq_etop_set_mac_address()
neterion/s2io: fix s2io_set_mac_addr() to prevent double checks
octeon: use dev_set_mac_address() instead of
octeon_mgmt_set_mac_address()
drivers/net/cris/eth_v10.c | 3 +--
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 2 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 13 ++++++++++---
drivers/net/ethernet/ethoc.c | 4 +---
drivers/net/ethernet/lantiq_etop.c | 4 ++--
drivers/net/ethernet/neterion/s2io.c | 5 +----
drivers/net/ethernet/octeon/octeon_mgmt.c | 6 ++----
include/linux/etherdevice.h | 2 +-
include/linux/netdevice.h | 7 +++++--
net/core/dev.c | 7 ++++++-
net/ethernet/eth.c | 12 +++++++++---
11 files changed, 39 insertions(+), 26 deletions(-)
--
1.7.8.3
Use dev_set_mac_address() instead of bcm_enet_set_mac_address() directly
to get validation checks for free.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index c7ca7ec..28184a8 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -858,7 +858,7 @@ static int bcm_enet_open(struct net_device *dev)
/* write device mac address */
memcpy(addr.sa_data, dev->dev_addr, ETH_ALEN);
- bcm_enet_set_mac_address(dev, &addr);
+ dev_set_mac_address(dev, &addr);
/* allocate rx dma ring */
size = priv->rx_ring_size * sizeof(struct bcm_enet_desc);
--
1.7.8.3
Validate the given MAC address directly in dev_set_mac_address()
if a .ndo_validate_addr function is available before calling
the .ndo_set_mac_address function.
Changed .ndo_validate_addr to take a second parameter containing
a sockaddr struct to be checked instead of the net_device dev_addr.
The behaviour of .ndo_validate_addr is now: if the second parameter
is NULL the net_device->dev_addr gets validate, if != NULL
the given parameter/sockaddr gets validated instead.
Removed is_valid_ether_addr() check from eth_mac_addr() since
this is now done in dev_set_mac_address(). Adapted eth_validate_addr()
to the changes.
Signed-off-by: Danny Kukawka <[email protected]>
---
include/linux/etherdevice.h | 2 +-
include/linux/netdevice.h | 7 +++++--
net/core/dev.c | 7 ++++++-
net/ethernet/eth.c | 12 +++++++++---
4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 8a18358..ee3091f 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -44,7 +44,7 @@ extern void eth_header_cache_update(struct hh_cache *hh,
const unsigned char *haddr);
extern int eth_mac_addr(struct net_device *dev, void *p);
extern int eth_change_mtu(struct net_device *dev, int new_mtu);
-extern int eth_validate_addr(struct net_device *dev);
+extern int eth_validate_addr(struct net_device *dev, void *addr);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f1b7d03..08186e3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -772,8 +772,10 @@ struct netdev_fcoe_hbainfo {
* needs to be changed. If this interface is not defined, the
* mac address can not be changed.
*
- * int (*ndo_validate_addr)(struct net_device *dev);
+ * int (*ndo_validate_addr)(struct net_device *dev, void *addr);
* Test if Media Access Control address is valid for the device.
+ * If addr is NULL the Media Access Control address of the device
+ * get validated, otherwise the MAC of the net_device.
*
* int (*ndo_do_ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd);
* Called when a user request an ioctl which can't be handled by
@@ -919,7 +921,8 @@ struct net_device_ops {
void (*ndo_set_rx_mode)(struct net_device *dev);
int (*ndo_set_mac_address)(struct net_device *dev,
void *addr);
- int (*ndo_validate_addr)(struct net_device *dev);
+ int (*ndo_validate_addr)(struct net_device *dev,
+ void *addr);
int (*ndo_do_ioctl)(struct net_device *dev,
struct ifreq *ifr, int cmd);
int (*ndo_set_config)(struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 763a0ed..b26a287 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1162,7 +1162,7 @@ static int __dev_open(struct net_device *dev)
set_bit(__LINK_STATE_START, &dev->state);
if (ops->ndo_validate_addr)
- ret = ops->ndo_validate_addr(dev);
+ ret = ops->ndo_validate_addr(dev, NULL);
if (!ret && ops->ndo_open)
ret = ops->ndo_open(dev);
@@ -4820,6 +4820,11 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
return -EINVAL;
if (!netif_device_present(dev))
return -ENODEV;
+ if (ops->ndo_validate_addr) {
+ err = ops->ndo_validate_addr(dev, sa);
+ if (err)
+ return err;
+ }
err = ops->ndo_set_mac_address(dev, sa);
if (!err)
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index a93af86..d25cc7a 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -285,8 +285,6 @@ int eth_mac_addr(struct net_device *dev, void *p)
if (netif_running(dev))
return -EBUSY;
- if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
/* if device marked as NET_ADDR_RANDOM, reset it */
dev->addr_assign_type &= ~NET_ADDR_RANDOM;
@@ -311,8 +309,16 @@ int eth_change_mtu(struct net_device *dev, int new_mtu)
}
EXPORT_SYMBOL(eth_change_mtu);
-int eth_validate_addr(struct net_device *dev)
+int eth_validate_addr(struct net_device *dev, void *addr)
{
+ struct sockaddr *saddr;
+
+ if (addr) {
+ saddr = addr;
+ if (!is_valid_ether_addr(saddr->sa_data))
+ return -EADDRNOTAVAIL;
+ }
+
if (!is_valid_ether_addr(dev->dev_addr))
return -EADDRNOTAVAIL;
--
1.7.8.3
Use dev_set_mac_address() instead of octeon_mgmt_set_mac_address() directly
to get validation checks for free.
Add .ndo_validate_addr = eth_validate_addr to enable validation.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/octeon/octeon_mgmt.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/octeon/octeon_mgmt.c b/drivers/net/ethernet/octeon/octeon_mgmt.c
index cd827ff..fb6553d 100644
--- a/drivers/net/ethernet/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/octeon/octeon_mgmt.c
@@ -551,9 +551,6 @@ static int octeon_mgmt_set_mac_address(struct net_device *netdev, void *addr)
{
struct sockaddr *sa = addr;
- if (!is_valid_ether_addr(sa->sa_data))
- return -EADDRNOTAVAIL;
-
memcpy(netdev->dev_addr, sa->sa_data, ETH_ALEN);
octeon_mgmt_set_rx_filtering(netdev);
@@ -768,7 +765,7 @@ static int octeon_mgmt_open(struct net_device *netdev)
cvmx_write_csr(CVMX_AGL_GMX_PRTX_CFG(port), prtx_cfg.u64);
memcpy(sa.sa_data, netdev->dev_addr, ETH_ALEN);
- octeon_mgmt_set_mac_address(netdev, &sa);
+ dev_set_mac_address(netdev, &sa);
octeon_mgmt_change_mtu(netdev, netdev->mtu);
@@ -1064,6 +1061,7 @@ static const struct net_device_ops octeon_mgmt_ops = {
.ndo_set_mac_address = octeon_mgmt_set_mac_address,
.ndo_do_ioctl = octeon_mgmt_ioctl,
.ndo_change_mtu = octeon_mgmt_change_mtu,
+ .ndo_validate_addr = eth_validate_addr,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = octeon_mgmt_poll_controller,
#endif
--
1.7.8.3
Add eth_validate_addr() to .ndo_validate_addr to get validation
checks in dev_set_mac_address() working. Remove
is_valid_ether_addr() from ethoc_set_mac_address() to prevent
double check.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/ethoc.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
index a381678..ca80f19 100644
--- a/drivers/net/ethernet/ethoc.c
+++ b/drivers/net/ethernet/ethoc.c
@@ -776,9 +776,6 @@ static int ethoc_set_mac_address(struct net_device *dev, void *addr)
struct ethoc *priv = netdev_priv(dev);
u8 *mac = (u8 *)addr;
- if (!is_valid_ether_addr(mac))
- return -EADDRNOTAVAIL;
-
ethoc_write(priv, MAC_ADDR0, (mac[2] << 24) | (mac[3] << 16) |
(mac[4] << 8) | (mac[5] << 0));
ethoc_write(priv, MAC_ADDR1, (mac[0] << 8) | (mac[1] << 0));
@@ -899,6 +896,7 @@ static const struct net_device_ops ethoc_netdev_ops = {
.ndo_change_mtu = ethoc_change_mtu,
.ndo_tx_timeout = ethoc_tx_timeout,
.ndo_start_xmit = ethoc_start_xmit,
+ .ndo_validate_addr = eth_validate_addr,
};
/**
--
1.7.8.3
Use dev_set_mac_address() instead of ltq_etop_set_mac_address() directly
to get validation checks for free.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/lantiq_etop.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 5dc9cbd..2419f51 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -650,11 +650,11 @@ ltq_etop_init(struct net_device *dev)
random_mac = true;
}
- err = ltq_etop_set_mac_address(dev, &mac);
+ err = dev_set_mac_address(dev, &mac);
if (err)
goto err_netdev;
- /* Set addr_assign_type here, ltq_etop_set_mac_address would reset it. */
+ /* Set addr_assign_type here, dev_set_mac_address would reset it. */
if (random_mac)
dev->addr_assign_type |= NET_ADDR_RANDOM;
--
1.7.8.3
Fix s2io_set_mac_addr() to prevent double validation checks from
dev_set_mac_address().
Don't use s2io_set_mac_addr() in s2io_io_resume() since it makes
no sense to copy netdev->dev_addr to itself. Use do_s2io_prog_unicast()
instead since this is what's needed and checked here.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/neterion/s2io.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index 22a8de0..cf7b3eb 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -5247,9 +5247,6 @@ static int s2io_set_mac_addr(struct net_device *dev, void *p)
{
struct sockaddr *addr = p;
- if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
-
memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
/* store the MAC address in CAM */
@@ -8658,7 +8655,7 @@ static void s2io_io_resume(struct pci_dev *pdev)
return;
}
- if (s2io_set_mac_addr(netdev, netdev->dev_addr) == FAILURE) {
+ if (do_s2io_prog_unicast(netdev, netdev->dev_addr) == FAILURE) {
s2io_card_down(sp);
pr_err("Can't restore mac addr after reset.\n");
return;
--
1.7.8.3
Use dev_set_mac_address() instead of e100_set_mac_address() directly
to get validation checks for free.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/cris/eth_v10.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/cris/eth_v10.c b/drivers/net/cris/eth_v10.c
index 7cb2785..ce6f3e6 100644
--- a/drivers/net/cris/eth_v10.c
+++ b/drivers/net/cris/eth_v10.c
@@ -377,8 +377,7 @@ etrax_ethernet_init(void)
}
/* set the default MAC address */
-
- e100_set_mac_address(dev, &default_mac);
+ dev_set_mac_address(dev, &default_mac);
/* Initialize speed indicator stuff. */
--
1.7.8.3
Adopted bnx2x_validate_addr() to changes in .ndo_validate_addr,
handle second parameter to be validated.
Signed-off-by: Danny Kukawka <[email protected]>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index b4afef6..d25ef1e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -10532,12 +10532,19 @@ static void poll_bnx2x(struct net_device *dev)
}
#endif
-static int bnx2x_validate_addr(struct net_device *dev)
+static int bnx2x_validate_addr(struct net_device *dev, void *addr)
{
struct bnx2x *bp = netdev_priv(dev);
+ struct sockaddr *saddr;
- if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
- return -EADDRNOTAVAIL;
+ if (addr) {
+ saddr = addr;
+ if (!bnx2x_is_valid_ether_addr(bp, saddr->sa_data))
+ return -EADDRNOTAVAIL;
+ } else {
+ if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
+ return -EADDRNOTAVAIL;
+ }
return 0;
}
--
1.7.8.3
Le 02/29/12 16:42, Danny Kukawka a écrit :
> Use dev_set_mac_address() instead of bcm_enet_set_mac_address() directly
> to get validation checks for free.
>
> Signed-off-by: Danny Kukawka<[email protected]>
Acked-by: Florian Fainelli <[email protected]>
> ---
> drivers/net/ethernet/broadcom/bcm63xx_enet.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
> index c7ca7ec..28184a8 100644
> --- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
> +++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
> @@ -858,7 +858,7 @@ static int bcm_enet_open(struct net_device *dev)
>
> /* write device mac address */
> memcpy(addr.sa_data, dev->dev_addr, ETH_ALEN);
> - bcm_enet_set_mac_address(dev,&addr);
> + dev_set_mac_address(dev,&addr);
>
> /* allocate rx dma ring */
> size = priv->rx_ring_size * sizeof(struct bcm_enet_desc);
On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> Validate the given MAC address directly in dev_set_mac_address()
> if a .ndo_validate_addr function is available before calling
> the .ndo_set_mac_address function.
>
> Changed .ndo_validate_addr to take a second parameter containing
> a sockaddr struct to be checked instead of the net_device dev_addr.
> The behaviour of .ndo_validate_addr is now: if the second parameter
> is NULL the net_device->dev_addr gets validate, if != NULL
> the given parameter/sockaddr gets validated instead.
>
> This patch series include adaptations for some drivers which
> use .ndo_set_mac_address functions directly - to prevent double
> checks and to enable validations via .ndo_validate_addr.
[...]
You have to do this as a single patch. The kernel and drivers should
still build at each stage.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On Mittwoch, 29. Februar 2012, you wrote:
> On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> > Validate the given MAC address directly in dev_set_mac_address()
> > if a .ndo_validate_addr function is available before calling
> > the .ndo_set_mac_address function.
> >
> > Changed .ndo_validate_addr to take a second parameter containing
> > a sockaddr struct to be checked instead of the net_device dev_addr.
> > The behaviour of .ndo_validate_addr is now: if the second parameter
> > is NULL the net_device->dev_addr gets validate, if != NULL
> > the given parameter/sockaddr gets validated instead.
> >
> > This patch series include adaptations for some drivers which
> > use .ndo_set_mac_address functions directly - to prevent double
> > checks and to enable validations via .ndo_validate_addr.
>
> [...]
>
> You have to do this as a single patch. The kernel and drivers should
> still build at each stage.
Okay. In this case only patch 1 and 2 needs to be merged into one commit/patch
since the other will even work with out patch one or two. I will send a new
version of the series as soon as I get more comments on the content.
Danny
On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> Adopted bnx2x_validate_addr() to changes in .ndo_validate_addr,
> handle second parameter to be validated.
>
> Signed-off-by: Danny Kukawka <[email protected]>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index b4afef6..d25ef1e 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -10532,12 +10532,19 @@ static void poll_bnx2x(struct net_device *dev)
> }
> #endif
>
> -static int bnx2x_validate_addr(struct net_device *dev)
> +static int bnx2x_validate_addr(struct net_device *dev, void *addr)
> {
> struct bnx2x *bp = netdev_priv(dev);
> + struct sockaddr *saddr;
>
> - if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> - return -EADDRNOTAVAIL;
> + if (addr) {
> + saddr = addr;
> + if (!bnx2x_is_valid_ether_addr(bp, saddr->sa_data))
> + return -EADDRNOTAVAIL;
> + } else {
> + if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> + return -EADDRNOTAVAIL;
> + }
> return 0;
> }
>
Isn't it preferred to calculate the correct address for test and then
call bnx2x_is_valid_ether_addr() at the end?
On Mittwoch, 29. Februar 2012, Danny Kukawka wrote:
> On Mittwoch, 29. Februar 2012, you wrote:
> > On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> > > Validate the given MAC address directly in dev_set_mac_address()
> > > if a .ndo_validate_addr function is available before calling
> > > the .ndo_set_mac_address function.
> > >
> > > Changed .ndo_validate_addr to take a second parameter containing
> > > a sockaddr struct to be checked instead of the net_device dev_addr.
> > > The behaviour of .ndo_validate_addr is now: if the second parameter
> > > is NULL the net_device->dev_addr gets validate, if != NULL
> > > the given parameter/sockaddr gets validated instead.
> > >
> > > This patch series include adaptations for some drivers which
> > > use .ndo_set_mac_address functions directly - to prevent double
> > > checks and to enable validations via .ndo_validate_addr.
> >
> > [...]
> >
> > You have to do this as a single patch. The kernel and drivers should
> > still build at each stage.
>
> Okay. In this case only patch 1 and 2 needs to be merged into one
> commit/patch since the other will even work with out patch one or two. I
> will send a new version of the series as soon as I get more comments on the
> content.
I have to send a new version anyway since I've accidently forgot to run
checkpatch.pl. Sorry.
Danny
On Mittwoch, 29. Februar 2012, Dmitry Kravkov wrote:
> On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> > Adopted bnx2x_validate_addr() to changes in .ndo_validate_addr,
> > handle second parameter to be validated.
> >
> > Signed-off-by: Danny Kukawka <[email protected]>
> > ---
> > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index b4afef6..d25ef1e
> > 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > @@ -10532,12 +10532,19 @@ static void poll_bnx2x(struct net_device *dev)
> > }
> > #endif
> >
> > -static int bnx2x_validate_addr(struct net_device *dev)
> > +static int bnx2x_validate_addr(struct net_device *dev, void *addr)
> > {
> > struct bnx2x *bp = netdev_priv(dev);
> > + struct sockaddr *saddr;
> >
> > - if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> > - return -EADDRNOTAVAIL;
> > + if (addr) {
> > + saddr = addr;
> > + if (!bnx2x_is_valid_ether_addr(bp, saddr->sa_data))
> > + return -EADDRNOTAVAIL;
> > + } else {
> > + if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> > + return -EADDRNOTAVAIL;
> > + }
> > return 0;
> > }
>
> Isn't it preferred to calculate the correct address for test and then
> call bnx2x_is_valid_ether_addr() at the end?
Do you mean something like this:
int eth_validate_addr(struct net_device *dev, void *addr)
{
u8 *vaddr;
if (addr)
vaddr = ((struct sockaddr *) addr)->sa_data;
else
vaddr = dev->dev_addr;
if (!is_valid_ether_addr(vaddr))
return -EADDRNOTAVAIL;
return 0;
}
On Wed, 2012-02-29 at 18:08 +0100, Danny Kukawka wrote:
> Do you mean something like this:
>
> int eth_validate_addr(struct net_device *dev, void *addr)
> {
> u8 *vaddr;
>
> if (addr)
> vaddr = ((struct sockaddr *) addr)->sa_data;
> else
> vaddr = dev->dev_addr;
>
> if (!is_valid_ether_addr(vaddr))
> return -EADDRNOTAVAIL;
>
> return 0;
> }
>
Exactly :)
On Wed, 2012-02-29 at 18:08 +0100, Danny Kukawka wrote:
> On Mittwoch, 29. Februar 2012, Dmitry Kravkov wrote:
> > On Wed, 2012-02-29 at 16:42 +0100, Danny Kukawka wrote:
> > > Adopted bnx2x_validate_addr() to changes in .ndo_validate_addr,
> > > handle second parameter to be validated.
> > >
> > > Signed-off-by: Danny Kukawka <[email protected]>
> > > ---
> > > drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 13 ++++++++++---
> > > 1 files changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > > b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index b4afef6..d25ef1e
> > > 100644
> > > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> > > @@ -10532,12 +10532,19 @@ static void poll_bnx2x(struct net_device *dev)
> > > }
> > > #endif
> > >
> > > -static int bnx2x_validate_addr(struct net_device *dev)
> > > +static int bnx2x_validate_addr(struct net_device *dev, void *addr)
> > > {
> > > struct bnx2x *bp = netdev_priv(dev);
> > > + struct sockaddr *saddr;
> > >
> > > - if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> > > - return -EADDRNOTAVAIL;
> > > + if (addr) {
> > > + saddr = addr;
> > > + if (!bnx2x_is_valid_ether_addr(bp, saddr->sa_data))
> > > + return -EADDRNOTAVAIL;
> > > + } else {
> > > + if (!bnx2x_is_valid_ether_addr(bp, dev->dev_addr))
> > > + return -EADDRNOTAVAIL;
> > > + }
> > > return 0;
> > > }
> >
> > Isn't it preferred to calculate the correct address for test and then
> > call bnx2x_is_valid_ether_addr() at the end?
>
> Do you mean something like this:
BUT:
> int eth_validate_addr(struct net_device *dev, void *addr)
> {
You still need to extract bp here:
struct bnx2x *bp = netdev_priv(dev);
> u8 *vaddr;
>
> if (addr)
> vaddr = ((struct sockaddr *) addr)->sa_data;
> else
> vaddr = dev->dev_addr;
>
> if (!is_valid_ether_addr(vaddr))
and call internal function here:
if (!bnx2x_is_valid_ether_addr(bp, vaddr))
> return -EADDRNOTAVAIL;
>
> return 0;
> }
>