All of lore.kernel.org
 help / color / mirror / Atom feed
From: jharan@bytemobile.com (Jeff Haran)
To: kernelnewbies@lists.kernelnewbies.org
Subject: locking in net_device_ops callbacks
Date: Fri, 24 Feb 2012 11:43:24 -0800	[thread overview]
Message-ID: <6F5DE7538AFCDA45A114F5E7510424A702FFFAA9@hq-exchange01.bytemobile.com> (raw)

Hi,

I was hoping somebody could enlighten me on how serialization of access
to data in struct net_device and associated netdev_priv() data works in
the callback functions registered in net_device_ops. For example, I am
looking at drivers/net/e1000/e1000_main.c in my 2.6.32 based kernel
tree. My understanding is this is a reference driver of sorts, so it
should be doing the "right thing".

This driver registers a callback for the ndo_set_mac_address callback
like this (much of what I think is not significant to the question code
replaced with "..." below for brevity):

static const struct net_device_ops e1000_netdev_ops = {
        ...
        .ndo_set_mac_address    = e1000_set_mac,
        ...
};

static int __devinit e1000_probe(struct pci_dev *pdev,
                                 const struct pci_device_id *ent)
{
        struct net_device *netdev;
       ....

        netdev = alloc_etherdev(sizeof(struct e1000_adapter));
        ....
        netdev->netdev_ops = &e1000_netdev_ops;
        ...
        err = register_netdev(netdev);
        ...
}

Most network drivers seem to follow more or less the same initialization
sequence.

When I look at the implementation of e1000_set_mac(), I see this:

static int e1000_set_mac(struct net_device *netdev, void *p)
{
        struct e1000_adapter *adapter = netdev_priv(netdev);
        struct e1000_hw *hw = &adapter->hw;
        struct sockaddr *addr = p;

        ...
        memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
        memcpy(hw->mac_addr, addr->sa_data, netdev->addr_len);
        ...

        return 0;
}

I can't help but notice there is no locking going on around the
memcpy()s of the passed in MAC address to the net_device dev_addr and
private data mac_addr fields.

I assume that e1000_set_mac() is typically called in process context in
response to some user space application like ifconfig or ip changing the
interface MAC address. I am also assuming that these dev_addr and
mac_addr fields are also referenced in other contexts. An Ethernet MAC
address is 6 bytes, so the memcpy()'s can't be atomic operations at
least on a 32 bit machine.

Shouldn't there have been some sort of lock taken before the memcpy()s
are executed so that other execution contexts won't see a partially
copied MAC address?

Is there some sort of lock taken higher up the call stack by the code
that calls these callback functions so that the callbacks themselves
don't have to do it explicitly?

I am writing a network device driver (modifying an existing one
actually) and am therefore trying to understand what kinds of explicit
locking my net_device_ops callbacks need to take in order to ensure
proper operation on an SMP system.

Thanks,

Jeff Haran

             reply	other threads:[~2012-02-24 19:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-24 19:43 Jeff Haran [this message]
2012-03-21 18:59 locking in net_device_ops callbacks Jeff Haran
2012-03-21 19:25 ` Stephen Hemminger
2012-03-21 20:38   ` Jeff Haran
2012-03-21 23:37     ` Francois Romieu
2012-03-21 19:58 ` Ben Hutchings
2012-03-21 23:00   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6F5DE7538AFCDA45A114F5E7510424A702FFFAA9@hq-exchange01.bytemobile.com \
    --to=jharan@bytemobile.com \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.