* Re: [Suggestion] net/atm : for sprintf, need check the total write length whether larger than a page.
From: Chas Williams (CONTRACTOR) @ 2012-12-04 3:46 UTC (permalink / raw)
To: Chen Gang; +Cc: David Miller, netdev
In-Reply-To: <50AC58BC.1020004@asianux.com>
In message <50AC58BC.1020004@asianux.com>,Chen Gang writes:
>in net/atm/atm_sysfs.c:
> suggest to check the write length whether larger than a page.
> the length of parameter buf is one page size (reference: fill_read_buffer at fs/sysfs/file.c)
> and the count of atm adresses are not limited (reference: atm_dev_ioctl -> atm_add_addr)
>
> thanks.
>
>gchen.
how about this as a possible fix?
atm: use scnprintf() instead of sprintf()
As reported by Chen Gang <gang.chen@asianux.com>, we should ensure there
is enough space when formatting the sysfs buffers.
Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>
---
net/atm/atm_sysfs.c | 40 +++++++++++++++-------------------------
1 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
index f49da58..350bf62 100644
--- a/net/atm/atm_sysfs.c
+++ b/net/atm/atm_sysfs.c
@@ -14,49 +14,45 @@ static ssize_t show_type(struct device *cdev,
struct device_attribute *attr, char *buf)
{
struct atm_dev *adev = to_atm_dev(cdev);
- return sprintf(buf, "%s\n", adev->type);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", adev->type);
}
static ssize_t show_address(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- int i;
-
- for (i = 0; i < (ESI_LEN - 1); i++)
- pos += sprintf(pos, "%02x:", adev->esi[i]);
- pos += sprintf(pos, "%02x\n", adev->esi[i]);
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
}
static ssize_t show_atmaddress(struct device *cdev,
struct device_attribute *attr, char *buf)
{
unsigned long flags;
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
struct atm_dev_addr *aaddr;
int bin[] = { 1, 2, 10, 6, 1 }, *fmt = bin;
- int i, j;
+ int i, j, count = 0;
spin_lock_irqsave(&adev->lock, flags);
list_for_each_entry(aaddr, &adev->local, entry) {
for (i = 0, j = 0; i < ATM_ESA_LEN; ++i, ++j) {
if (j == *fmt) {
- pos += sprintf(pos, ".");
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count, ".");
++fmt;
j = 0;
}
- pos += sprintf(pos, "%02x",
- aaddr->addr.sas_addr.prv[i]);
+ count += scnprintf(buf + count,
+ PAGE_SIZE - count, "%02x",
+ aaddr->addr.sas_addr.prv[i]);
}
- pos += sprintf(pos, "\n");
+ count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
}
spin_unlock_irqrestore(&adev->lock, flags);
- return pos - buf;
+ return count;
}
static ssize_t show_atmindex(struct device *cdev,
@@ -64,25 +60,21 @@ static ssize_t show_atmindex(struct device *cdev,
{
struct atm_dev *adev = to_atm_dev(cdev);
- return sprintf(buf, "%d\n", adev->number);
+ return scnprintf(buf, PAGE_SIZE, "%d\n", adev->number);
}
static ssize_t show_carrier(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- pos += sprintf(pos, "%d\n",
- adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
-
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
+ adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
}
static ssize_t show_link_rate(struct device *cdev,
struct device_attribute *attr, char *buf)
{
- char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
int link_rate;
@@ -100,9 +92,7 @@ static ssize_t show_link_rate(struct device *cdev,
default:
link_rate = adev->link_rate * 8 * 53;
}
- pos += sprintf(pos, "%d\n", link_rate);
-
- return pos - buf;
+ return scnprintf(buf, PAGE_SIZE, "%d\n", link_rate);
}
static DEVICE_ATTR(address, S_IRUGO, show_address, NULL);
--
1.7.7.6
^ permalink raw reply related
* Re: [net-next rfc v7 1/3] virtio-net: separate fields of sending/receiving queue from virtnet_info
From: Rusty Russell @ 2012-12-04 3:43 UTC (permalink / raw)
To: Jason Wang
Cc: krkumar2, kvm, mst, netdev, linux-kernel, virtualization,
bhutchings, jwhan, shiyer
In-Reply-To: <3524590.ZWGua7A8ne@jason-thinkpad-t430s>
Jason Wang <jasowang@redhat.com> writes:
> On Monday, December 03, 2012 12:25:42 PM Rusty Russell wrote:
>> > +
>> > + /* Work struct for refilling if we run low on memory. */
>> > + struct delayed_work refill;
>>
>> I can't really see the justificaiton for a refill per queue. Just have
>> one work iterate all the queues if it happens, unless it happens often
>> (in which case, we need to look harder at this anyway).
>
> But during this kind of iteration, we may need enable/disable the napi
> regardless of whether the receive queue has lots to be refilled. This may add
> extra latency.
Sure, but does it actually happen? We only use the work when we run out
of memory. If this happens in normal behaviour we need to change
something else...
Thanks,
Rusty.
^ permalink raw reply
* WARNING!!! VIRUS DETECTED AND SPY, UPDATE NOW!!!
From: Webmail System Administrator @ 2012-12-04 3:29 UTC (permalink / raw)
your webmail account need to be updated. you are advise to use the weblink
below to update now.
https://docs.google.com/a/blumail.org/spreadsheet/viewform?formkey=dGdSbktDQVN0TldpUGVwWmY3V1RfRHc6MQ
Thank you for using our email.
Copyright ©2012 Email Helpdesk Centre.
^ permalink raw reply
* [PATCH] dev_change_net_namespace: send a KOBJ_REMOVED/KOBJ_ADD
From: Serge Hallyn @ 2012-12-04 2:17 UTC (permalink / raw)
To: linux-kernel, netdev, Eric W. Biederman, Daniel Lezcano
When a new nic is created in namespace ns1, the kernel sends a KOBJ_ADD uevent
to ns1. When the nic is moved to ns2, we only send a KOBJ_MOVE to ns2, and
nothing to ns1.
This patch changes that behavior so that when moving a nic from ns1 to ns2, we
send a KOBJ_REMOVED to ns1 and KOBJ_ADD to ns2. (The KOBJ_MOVE is still
sent to ns2).
The effects of this can be seen when starting and stopping containers in
an upstart based host. Lxc will create a pair of veth nics, the kernel
sends KOBJ_ADD, and upstart starts network-instance jobs for each. When
one nic is moved to the container, because no KOBJ_REMOVED event is
received, the network-instance job for that veth never goes away. This
was reported at https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1065589
With this patch the networ-instance jobs properly go away.
The other oddness solved here is that if a nic is passed into a running
upstart-based container, without this patch no network-instance job is
started in the container. But when the container creates a new nic
itself (ip link add new type veth) then network-interface jobs are
created. With this patch, behavior comes in line with a regular host.
v2: also send KOBJ_ADD to new netns. There will then be a
_MOVE event from the device_rename() call, but that should
be innocuous.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
---
net/core/dev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index e2215ee..2c43aaf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6172,6 +6172,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
dev_uc_flush(dev);
dev_mc_flush(dev);
+ /* Send a netdev-removed uevent to the old namespace */
+ kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
+
/* Actually switch the network namespace */
dev_net_set(dev, net);
@@ -6183,6 +6186,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
dev->iflink = dev->ifindex;
}
+ /* Send a netdev-add uevent to the new namespace */
+ kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
+
/* Fixup kobjects */
err = device_rename(&dev->dev, dev->name);
WARN_ON(err);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] bridge: implement multicast fast leave
From: Herbert Xu @ 2012-12-04 1:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Cong Wang, netdev, bridge, David S. Miller
In-Reply-To: <20121203075316.0b1da39d@nehalam.linuxnetplumber.net>
On Mon, Dec 03, 2012 at 07:53:16AM -0800, Stephen Hemminger wrote:
> On Mon, 3 Dec 2012 22:36:03 +0800
> Cong Wang <amwang@redhat.com> wrote:
>
> > Fast leave allows bridge to immediately stops the multicast
> > traffic on the port receives IGMP Leave when IGMP snooping is enabled,
> > no timeouts are observed.
> >
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: Stephen Hemminger <shemminger@vyatta.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Signed-off-by: Cong Wang <amwang@redhat.com>
>
> I like the feature, and it looks like an oversight in the initial design.
> Why is this not the default, adding more options obscures it.
If the port has a bridge on it then you're toast. I think this
should be a per-port option.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re[2]: [PATCH] vxlan: Fix error that was resulting in VXLAN MTU size being 10 bytes too large
From: Naoto MATSUMOTO @ 2012-12-04 0:48 UTC (permalink / raw)
To: Joseph Glanville
Cc: Stephen Hemminger, David Miller, alexander.h.duyck, netdev
In-Reply-To: <CAOzFzEh0X3CcjBYWVDAtu7=jRSc1_P2OQSF_YWjnBSeugo3W6A@mail.gmail.com>
Hi all
Sharing my testlab resut for you ;-)
A First Look At VXLAN over Infiniband Network On Linux 3.7-rc7
http://slidesha.re/TsCKWc
plz enjyoi it.
--
Naoto
On Tue, 4 Dec 2012 02:26:18 +1100
Joseph Glanville <joseph.glanville@orionvm.com.au> wrote:
> On 20 November 2012 03:03, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > On Mon, 19 Nov 2012 22:33:50 +1100
> > Joseph Glanville <joseph.glanville@orionvm.com.au> wrote:
> >
> >> On 14 November 2012 08:33, Stephen Hemminger <shemminger@vyatta.com> wrote:
> >> > On Tue, 13 Nov 2012 14:37:19 -0500 (EST)
> >> > David Miller <davem@davemloft.net> wrote:
> >> >
> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> Date: Fri, 09 Nov 2012 15:35:24 -0800
> >> >>
> >> >> > This change fixes an issue I found where VXLAN frames were fragmented when
> >> >> > they were up to the VXLAN MTU size. I root caused the issue to the fact that
> >> >> > the headroom was 4 + 20 + 8 + 8. This math doesn't appear to be correct
> >> >> > because we are not inserting a VLAN header, but instead a 2nd Ethernet header.
> >> >> > As such the math for the overhead should be 20 + 8 + 8 + 14 to account for the
> >> >> > extra headers that are inserted for VXLAN.
> >> >> >
> >> >> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >>
> >> >> Applied, thanks for the detailed commit message.
> >> >
> >> > Probably need smarter code there to look at header length requirement
> >> > of underlying device as well, maybe someone will be perverse and runn
> >> > vxlan over a tunnel or IPoIB.
> >>
> >> Forgive my ignorance but why would running VXLAN on IPoIB require
> >> special header handling? (and would it work or behave strangely?)
> >>
> >> I was planning on giving this a go when 3.7 is released but I might do
> >> that sooner if problems are anticipated.
> >>
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> >> > the body of a message to majordomo@vger.kernel.org
> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> >> Joseph.
> >>
> >
> > Some lower layers require bigger (or smaller headers). As it was, vxlan
> > was only allocating skb with a fixed amount of headroom. This would lead to
> > lower layers having to copy the skb.
> >
> > My suggestion has already been addressed by a later patch.
>
> Hi,
>
> I have tested VXLAN on IPoIB and it works perfectly. :)
>
> Joseph.
>
>
> --
> CTO | Orion Virtualisation Solutions | www.orionvm.com.au
> Phone: 1300 56 99 52 | Mobile: 0428 754 846
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
SAKURA Internet Inc. / Senior Researcher
Naoto MATSUMOTO <n-matsumoto@sakura.ad.jp>
SAKURA Research Center <http://research.sakura.ad.jp/>
^ permalink raw reply
* Re: [net-next rfc v7 3/3] virtio-net: change the number of queues through ethtool
From: Ben Hutchings @ 2012-12-04 0:22 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: krkumar2, kvm, netdev, linux-kernel, virtualization, jwhan,
shiyer
In-Reply-To: <20121203112507.GE26167@redhat.com>
On Mon, 2012-12-03 at 13:25 +0200, Michael S. Tsirkin wrote:
> On Mon, Dec 03, 2012 at 02:09:28PM +0800, Jason Wang wrote:
> > On Sunday, December 02, 2012 06:09:06 PM Michael S. Tsirkin wrote:
> > > On Tue, Nov 27, 2012 at 06:16:00PM +0800, Jason Wang wrote:
> > > > This patch implement the {set|get}_channels method of ethool to allow user
> > > > to change the number of queues dymaically when the device is running.
> > > > This would let the user to configure it on demand.
> > > >
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > ---
> > > >
> > > > drivers/net/virtio_net.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > 1 files changed, 41 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index bcaa6e5..f08ec2a 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -1578,10 +1578,51 @@ static struct virtio_driver virtio_net_driver = {
> > > >
> > > > #endif
> > > > };
> > > >
> > > > +/* TODO: Eliminate OOO packets during switching */
> > > > +static int virtnet_set_channels(struct net_device *dev,
> > > > + struct ethtool_channels *channels)
> > > > +{
> > > > + struct virtnet_info *vi = netdev_priv(dev);
> > > > + u16 queue_pairs = channels->combined_count;
>
> by the way shouldn't this be combined_count / 2?
>
> And below channels->max_combined = vi->max_queue_pairs * 2; ?
[...]
In this ethtool API, 'channel' means an IRQ and set of queues that
trigger it. So each ethtool-channel will correspond to one queue-pair
and not one virtio channel.
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.
^ permalink raw reply
* Re: [PATCH] net: ICMPv6 packets transmitted on wrong interface if nfmark is mangled
From: Pablo Neira Ayuso @ 2012-12-03 23:52 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Dries De Winter, David Miller, kaber, netdev, netfilter-devel
In-Reply-To: <alpine.LNX.2.01.1212040037240.2816@nerf07.vanv.qr>
On Tue, Dec 04, 2012 at 12:38:25AM +0100, Jan Engelhardt wrote:
>
> On Monday 2012-12-03 22:31, Dries De Winter wrote:
> >
> >Not fixing this means that skb->mark is unavailable for use on ICMPv6
> >packets because it will inevitably put those packets on the wrong
> >interface. [...]
> >
> >I use skb->mark for QoS, not for routing so I don't expect
> >the outgoing interface to be affected by my markers.
>
> Why would it do that, if one has no routes joined to a fwmark NNN
> routing rule?
iptables_mangle assumes that ip_route_me_harder needs to be called if
the mark has changed.
^ permalink raw reply
* Re: [PATCH] net: ICMPv6 packets transmitted on wrong interface if nfmark is mangled
From: Jan Engelhardt @ 2012-12-03 23:38 UTC (permalink / raw)
To: Dries De Winter; +Cc: David Miller, pablo, kaber, netdev, netfilter-devel
In-Reply-To: <CA+e04fjWMDE9xEApysFRprZDBdM3Ya2RHrxtoau7i+fxzGT8CQ@mail.gmail.com>
On Monday 2012-12-03 22:31, Dries De Winter wrote:
>
>Not fixing this means that skb->mark is unavailable for use on ICMPv6
>packets because it will inevitably put those packets on the wrong
>interface. [...]
>I use skb->mark for QoS, not for routing so I don't expect
>the outgoing interface to be affected by my markers.
Why would it do that, if one has no routes joined to a fwmark NNN
routing rule?
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Use eth_mac_addr() instead of duplicating it
From: Thomas Graf @ 2012-12-03 23:16 UTC (permalink / raw)
To: Jesse Gross; +Cc: davem, netdev, dev
In-Reply-To: <CAEP_g=_sAOsQAXQXELDUAc3vFPgXAYbQUG6THfcsuNvXg7+M8A@mail.gmail.com>
On 12/03/12 at 02:55pm, Jesse Gross wrote:
> If you send patches like this to two different trees then it will
> result in merge conflicts later. Please just wait a few days; as you
> say, it's a trivial patch.
I CC'ed dev@openvswitch.org on both patch submissions as well so if
davem merges it you will know.
It's no the lack of response to this patch but the discontinuation
of discussion on the previous patchset without any reason that gave
me the impression of being ignored. When I feel ignored, I go
somehwere else.
Besides, it would have taken less time to just apply the patch than
to have this discussion.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Use eth_mac_addr() instead of duplicating it
From: Jesse Gross @ 2012-12-03 22:55 UTC (permalink / raw)
To: Thomas Graf
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121203224721.GC14494-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>
On Mon, Dec 3, 2012 at 2:47 PM, Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org> wrote:
> On 12/03/12 at 02:36pm, Jesse Gross wrote:
>> On Mon, Dec 3, 2012 at 2:17 PM, Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org> wrote:
>> >
>> > bonus: if we ever are to use IFF_LIVE_ADDR_CHANGE for
>> > anything further than to check availability in eth_mac_addr(),
>> > Open vSwitch will be ready for that.
>> >
>> > Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
>>
>> There's no need to send this patch twice (especially to a different
>> set of people). I'm currently reviewing a large patchset that was
>> submitted before yours.
>
> It's a trivial patch, you seemed busy and we might as well make use
> of the open net-next window. I see nothing wrong with that.
If you send patches like this to two different trees then it will
result in merge conflicts later. Please just wait a few days; as you
say, it's a trivial patch.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Use eth_mac_addr() instead of duplicating it
From: Thomas Graf @ 2012-12-03 22:47 UTC (permalink / raw)
To: Jesse Gross; +Cc: davem, netdev, dev
In-Reply-To: <CAEP_g=8+V=LN--ZkAgRHysUWwNJH1O=YDevbLH846nGZQkUpUA@mail.gmail.com>
On 12/03/12 at 02:36pm, Jesse Gross wrote:
> On Mon, Dec 3, 2012 at 2:17 PM, Thomas Graf <tgraf@suug.ch> wrote:
> >
> > bonus: if we ever are to use IFF_LIVE_ADDR_CHANGE for
> > anything further than to check availability in eth_mac_addr(),
> > Open vSwitch will be ready for that.
> >
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
>
> There's no need to send this patch twice (especially to a different
> set of people). I'm currently reviewing a large patchset that was
> submitted before yours.
It's a trivial patch, you seemed busy and we might as well make use
of the open net-next window. I see nothing wrong with that.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Use eth_mac_addr() instead of duplicating it
From: Jesse Gross @ 2012-12-03 22:36 UTC (permalink / raw)
To: Thomas Graf
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20121203221732.GA14494-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org>
On Mon, Dec 3, 2012 at 2:17 PM, Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org> wrote:
>
> bonus: if we ever are to use IFF_LIVE_ADDR_CHANGE for
> anything further than to check availability in eth_mac_addr(),
> Open vSwitch will be ready for that.
>
> Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
There's no need to send this patch twice (especially to a different
set of people). I'm currently reviewing a large patchset that was
submitted before yours.
^ permalink raw reply
* [PATCH net-next] openvswitch: Avoid useless holes in struct vport
From: Thomas Graf @ 2012-12-03 22:24 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
Having the 16bit port_no in between a set of pointers creates
an unwanted and useless hole in the struct.
Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
---
net/openvswitch/vport.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
index 3f7961e..aee7d43 100644
--- a/net/openvswitch/vport.h
+++ b/net/openvswitch/vport.h
@@ -68,10 +68,10 @@ struct vport_err_stats {
/**
* struct vport - one port within a datapath
* @rcu: RCU callback head for deferred destruction.
- * @port_no: Index into @dp's @ports array.
* @dp: Datapath to which this port belongs.
* @upcall_portid: The Netlink port to use for packets received on this port that
* miss the flow table.
+ * @port_no: Index into @dp's @ports array.
* @hash_node: Element in @dev_table hash table in vport.c.
* @dp_hash_node: Element in @datapath->ports hash table in datapath.c.
* @ops: Class structure.
@@ -81,9 +81,9 @@ struct vport_err_stats {
*/
struct vport {
struct rcu_head rcu;
- u16 port_no;
struct datapath *dp;
u32 upcall_portid;
+ u16 port_no;
struct hlist_node hash_node;
struct hlist_node dp_hash_node;
^ permalink raw reply related
* [PATCH net-next] openvswitch: Use eth_mac_addr() instead of duplicating it
From: Thomas Graf @ 2012-12-03 22:17 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
bonus: if we ever are to use IFF_LIVE_ADDR_CHANGE for
anything further than to check availability in eth_mac_addr(),
Open vSwitch will be ready for that.
Signed-off-by: Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org>
---
net/openvswitch/vport-internal_dev.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 5d460c3..90816c7 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -63,17 +63,6 @@ static struct rtnl_link_stats64 *internal_dev_get_stats(struct net_device *netde
return stats;
}
-static int internal_dev_mac_addr(struct net_device *dev, void *p)
-{
- struct sockaddr *addr = p;
-
- if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
- dev->addr_assign_type &= ~NET_ADDR_RANDOM;
- memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
- return 0;
-}
-
/* Called with rcu_read_lock_bh. */
static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
{
@@ -127,7 +116,7 @@ static const struct net_device_ops internal_dev_netdev_ops = {
.ndo_open = internal_dev_open,
.ndo_stop = internal_dev_stop,
.ndo_start_xmit = internal_dev_xmit,
- .ndo_set_mac_address = internal_dev_mac_addr,
+ .ndo_set_mac_address = eth_mac_addr,
.ndo_change_mtu = internal_dev_change_mtu,
.ndo_get_stats64 = internal_dev_get_stats,
};
@@ -139,6 +128,7 @@ static void do_setup(struct net_device *netdev)
netdev->netdev_ops = &internal_dev_netdev_ops;
netdev->priv_flags &= ~IFF_TX_SKB_SHARING;
+ netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
netdev->destructor = internal_dev_destructor;
SET_ETHTOOL_OPS(netdev, &internal_dev_ethtool_ops);
netdev->tx_queue_len = 0;
^ permalink raw reply related
* Re: [PATCH] net: ICMPv6 packets transmitted on wrong interface if nfmark is mangled
From: David Miller @ 2012-12-03 22:06 UTC (permalink / raw)
To: dries.dewinter; +Cc: pablo, kaber, netdev, netfilter-devel
In-Reply-To: <CA+e04fjWMDE9xEApysFRprZDBdM3Ya2RHrxtoau7i+fxzGT8CQ@mail.gmail.com>
From: Dries De Winter <dries.dewinter@gmail.com>
Date: Mon, 3 Dec 2012 22:31:51 +0100
> Not fixing this means that skb->mark is unavailable for use on ICMPv6
> packets because it will inevitably put those packets on the wrong
> interface.
Maybe this suggests that a better fix is to simply explicitly check
for protocol ICMPV6 in ip6_route_me_harder().
^ permalink raw reply
* Re: [PATCH 2/4 net-next] tg3: PTP - Implement the ptp api and ethtool functions
From: Nithin Nayak Sujir @ 2012-12-03 21:52 UTC (permalink / raw)
To: Richard Cochran; +Cc: Michael Chan, davem, netdev
In-Reply-To: <20121203185157.GB2531@netboy.at.omicron.at>
On 12/03/2012 10:51 AM, Richard Cochran wrote:
> On Sun, Dec 02, 2012 at 07:42:49PM -0800, Michael Chan wrote:
>> From: Matt Carlson <mcarlson@broadcom.com>
>>
>> This patch updates the ptp_caps structure with implementation functions.
>> All the basic clock operations as described in
>> Documentation/ptp/ptp.txt are supported.
>>
>> Frequency adjustment is performed using hardware with a 24 bit
>> accumulator and a programmable correction value. On each clk, the
>> correction value gets added to the accumulator and when it overflows,
>> the time counter is incremented/decremented and the accumulator reset.
>>
>> So conversion from ppb to correction value is
>> ppb * (1 << 24) / 1000000000
> Are you sure about this? I don't know your hardware, but the others
> ones with an addend and an accumulator work differently. Usually there
> is a default addend based on the frequency ratio between the input
> clock and the divided nominal clock. Then the ppb is applied to the
> default addend.
>
> For example, see how tmr_add is calculated in
>
> Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
Yes, the hardware does seem to be different from what you describe but I think
the conversion is right. I tested this with ptp4l in a back-to-back
configuration and observed convergence of the master offset to ~0. Without this
code, the offset keeps increasing.
>
>> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
>> Signed-off-by: Michael Chan <mchan@broadcom.com>
>> ---
>> drivers/net/ethernet/broadcom/tg3.c | 125 ++++++++++++++++++++++++++++++++++-
>> 1 files changed, 123 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
>> index 38047a9..a54d194 100644
>> --- a/drivers/net/ethernet/broadcom/tg3.c
>> +++ b/drivers/net/ethernet/broadcom/tg3.c
>> @@ -5519,6 +5519,14 @@ static int tg3_setup_phy(struct tg3 *tp, int force_reset)
>> return err;
>> }
>>
>> +
>> +static u64 tg3_refclk_read(struct tg3 *tp)
>> +{
>> + u64 stamp = tr32(TG3_EAV_REF_CLCK_LSB);
>> +
>> + return stamp | (u64) tr32(TG3_EAV_REF_CLCK_MSB) << 32;
>> +}
>> +
>> static void tg3_refclk_write(struct tg3 *tp, u64 newval)
>> {
>> tw32(TG3_EAV_REF_CLCK_CTL, TG3_EAV_REF_CLCK_CTL_STOP);
>> @@ -5527,14 +5535,127 @@ static void tg3_refclk_write(struct tg3 *tp, u64 newval)
>> tw32_f(TG3_EAV_REF_CLCK_CTL, TG3_EAV_REF_CLCK_CTL_RESUME);
>> }
>>
>> +static inline void tg3_full_lock(struct tg3 *tp, int irq_sync);
>> +static inline void tg3_full_unlock(struct tg3 *tp);
>> +static int tg3_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
>> +{
>> + struct tg3 *tp = netdev_priv(dev);
>> +
>> + info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
>> + SOF_TIMESTAMPING_RX_SOFTWARE |
>> + SOF_TIMESTAMPING_SOFTWARE |
>> + SOF_TIMESTAMPING_TX_HARDWARE |
>> + SOF_TIMESTAMPING_RX_HARDWARE |
>> + SOF_TIMESTAMPING_RAW_HARDWARE;
>> +
>> + if (tp->ptp_clock)
>> + info->phc_index = ptp_clock_index(tp->ptp_clock);
>> + else
>> + info->phc_index = -1;
>> +
>> + info->tx_types = (1 << HWTSTAMP_TX_OFF) |
>> + (1 << HWTSTAMP_TX_ON);
>> +
>> + info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
>> + (1 << HWTSTAMP_FILTER_ALL);
>> + return 0;
>> +}
>> +
>> +static int tg3_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
>> +{
>> + struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
>> + bool neg_adj = false;
>> + u32 correction = 0;
>> +
>> + if (ppb < 0) {
>> + neg_adj = true;
>> + ppb = -ppb;
>> + }
>> +
>> + /* Frequency adjustment is performed using hardware with a 24 bit
>> + * accumulator and a programmable correction value. On each clk, the
>> + * correction value gets added to the accumulator and when it
>> + * overflows, the time counter is incremented/decremented.
>> + *
>> + * So conversion from ppb to correction value is
>> + * ppb * (1 << 24) / 1000000000
>> + */
>> + correction = div_u64((u64)ppb * (1 << 24), 1000000000ULL) &
>> + TG3_EAV_REF_CLK_CORRECT_MASK;
>> +
>> + tg3_full_lock(tp, 0);
>> +
>> + if (correction)
>> + tw32(TG3_EAV_REF_CLK_CORRECT_CTL,
>> + TG3_EAV_REF_CLK_CORRECT_EN |
>> + (neg_adj ? TG3_EAV_REF_CLK_CORRECT_NEG : 0) | correction);
>> + else
>> + tw32(TG3_EAV_REF_CLK_CORRECT_CTL, 0);
>> +
>> + tg3_full_unlock(tp);
>> +
>> + return 0;
>> +}
>> +
>> +static int tg3_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>> +{
>> + struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
>> + tp->ptp_adjust += delta;
> This 'ptp_adjust' should be placed under the lock. This races with the
> reader method.
>
>> + return 0;
>> +}
>> +
>> +static int tg3_ptp_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
>> +{
>> + u64 ns;
>> + u32 remainder;
>> + struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
>> +
>> + tg3_full_lock(tp, 0);
>> + ns = tg3_refclk_read(tp);
>> + tg3_full_unlock(tp);
>> + ns += tp->ptp_adjust;
>> +
>> + ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder);
>> + ts->tv_nsec = remainder;
>> +
>> + return 0;
>> +}
>> +
>> +static int tg3_ptp_settime(struct ptp_clock_info *ptp,
>> + const struct timespec *ts)
>> +{
>> + u64 ns;
>> + struct tg3 *tp = container_of(ptp, struct tg3, ptp_info);
>> +
>> + ns = timespec_to_ns(ts);
>> +
>> + tg3_full_lock(tp, 0);
>> + tg3_refclk_write(tp, ns);
>> + tg3_full_unlock(tp);
>> + tp->ptp_adjust = 0;
>> +
>> + return 0;
>> +}
>> +
>> +static int tg3_ptp_enable(struct ptp_clock_info *ptp,
>> + struct ptp_clock_request *rq, int on)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> static const struct ptp_clock_info tg3_ptp_caps = {
>> .owner = THIS_MODULE,
>> .name = "",
>> - .max_adj = 0,
>> + .max_adj = 250000000,
>> .n_alarm = 0,
>> .n_ext_ts = 0,
>> .n_per_out = 0,
>> .pps = 0,
>> + .adjfreq = tg3_ptp_adjfreq,
>> + .adjtime = tg3_ptp_adjtime,
>> + .gettime = tg3_ptp_gettime,
>> + .settime = tg3_ptp_settime,
>> + .enable = tg3_ptp_enable,
> These are missing from patch #1.
>
>> };
>>
>> static void tg3_ptp_init(struct tg3 *tp)
> Thanks,
> Richard
>
^ permalink raw reply
* Re: [PATCH 1/4 net-next] tg3: PTP - Add header definitions, initialization and hw access functions.
From: Nithin Nayak Sujir @ 2012-12-03 21:49 UTC (permalink / raw)
To: Richard Cochran; +Cc: Michael Chan, davem, netdev
In-Reply-To: <20121203182338.GA2531@netboy.at.omicron.at>
Hi Richard,
Thanks for your comments.
On 12/03/2012 10:23 AM, Richard Cochran wrote:
> Please put me on CC for any PTP related patches.
> I have a few comments, below.
>
> On Sun, Dec 02, 2012 at 07:42:48PM -0800, Michael Chan wrote:
>> From: Matt Carlson <mcarlson@broadcom.com>
>>
>> This patch adds code to register/unregister the ptp clock and write
>> the reference clock. If a chip reset is performed, the hwclock is
>> reinitialized with the adjusted kernel time
>>
>> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
>> Signed-off-by: Michael Chan <mchan@broadcom.com>
>> ---
>> drivers/net/ethernet/broadcom/Kconfig | 1 +
>> drivers/net/ethernet/broadcom/tg3.c | 84 +++++++++++++++++++++++++++++++--
>> drivers/net/ethernet/broadcom/tg3.h | 60 ++++++++++++++++++++++-
>> 3 files changed, 137 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
>> index 4bd416b..f552673 100644
>> --- a/drivers/net/ethernet/broadcom/Kconfig
>> +++ b/drivers/net/ethernet/broadcom/Kconfig
>> @@ -102,6 +102,7 @@ config TIGON3
>> depends on PCI
>> select PHYLIB
>> select HWMON
>> + select PTP_1588_CLOCK
>> ---help---
>> This driver supports Broadcom Tigon3 based gigabit Ethernet cards.
>>
>> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
>> index 5cc976d..38047a9 100644
>> --- a/drivers/net/ethernet/broadcom/tg3.c
>> +++ b/drivers/net/ethernet/broadcom/tg3.c
>> @@ -54,6 +54,9 @@
>> #include <asm/byteorder.h>
>> #include <linux/uaccess.h>
>>
>> +#include <uapi/linux/net_tstamp.h>
>> +#include <linux/ptp_clock_kernel.h>
>> +
>> #ifdef CONFIG_SPARC
>> #include <asm/idprom.h>
>> #include <asm/prom.h>
>> @@ -5516,6 +5519,57 @@ static int tg3_setup_phy(struct tg3 *tp, int force_reset)
>> return err;
>> }
>>
>> +static void tg3_refclk_write(struct tg3 *tp, u64 newval)
>> +{
>> + tw32(TG3_EAV_REF_CLCK_CTL, TG3_EAV_REF_CLCK_CTL_STOP);
>> + tw32(TG3_EAV_REF_CLCK_LSB, newval & 0xffffffff);
>> + tw32(TG3_EAV_REF_CLCK_MSB, newval >> 32);
>> + tw32_f(TG3_EAV_REF_CLCK_CTL, TG3_EAV_REF_CLCK_CTL_RESUME);
>> +}
>> +
>> +static const struct ptp_clock_info tg3_ptp_caps = {
>> + .owner = THIS_MODULE,
>> + .name = "",
> Please use a static name here, something related to the driver, like
> "tigon3 clock" perhaps. There used to be drivers doing other things
> with this, but now the kernel doc from ptp_clock_kernel.h says:
>
> @name: A short "friendly name" to identify the clock and to
> help distinguish PHY based devices from MAC based ones.
> The string is not meant to be a unique id.
>
>> + .max_adj = 0,
>> + .n_alarm = 0,
>> + .n_ext_ts = 0,
>> + .n_per_out = 0,
>> + .pps = 0,
> You have left the methods as a bunch of NULL pointers. This will not
> fly, since someone might land on this commit during a bisect. In
> general, every patch in a series should result in a working kernel.
>
>> +};
>> +
>> +static void tg3_ptp_init(struct tg3 *tp)
>> +{
>> + if (!tg3_flag(tp, PTP_CAPABLE))
>> + return;
>> +
>> + /* Initialize the hardware clock to the system time. */
>> + tg3_refclk_write(tp, ktime_to_ns(ktime_get_real()));
>> + tp->ptp_adjust = 0;
>> +
>> + tp->ptp_info = tg3_ptp_caps;
>> + strncpy(tp->ptp_info.name, tp->dev->name, IFNAMSIZ);
>> +}
>> +
>> +static void tg3_ptp_resume(struct tg3 *tp)
>> +{
>> + if (!tg3_flag(tp, PTP_CAPABLE))
>> + return;
>> +
>> + tg3_refclk_write(tp, ktime_to_ns(ktime_get_real()) + tp->ptp_adjust);
>> + tp->ptp_adjust = 0;
>> +}
>> +
>> +static void tg3_ptp_fini(struct tg3 *tp)
>> +{
>> + if (!tg3_flag(tp, PTP_CAPABLE) ||
>> + !tp->ptp_clock)
> Overzealous line break.
>
>> + return;
>> +
>> + ptp_clock_unregister(tp->ptp_clock);
>> + tp->ptp_clock = NULL;
>> + tp->ptp_adjust = 0;
>> +}
>> +
>> static inline int tg3_irq_sync(struct tg3 *tp)
>> {
>> return tp->irq_sync;
>> @@ -6527,6 +6581,8 @@ static inline void tg3_netif_stop(struct tg3 *tp)
>>
>> static inline void tg3_netif_start(struct tg3 *tp)
>> {
>> + tg3_ptp_resume(tp);
>> +
>> /* NOTE: unconditional netif_tx_wake_all_queues is only
>> * appropriate so long as all callers are assured to
>> * have free tx slots (such as after tg3_init_hw)
>> @@ -10364,7 +10420,8 @@ static void tg3_ints_fini(struct tg3 *tp)
>> tg3_flag_clear(tp, ENABLE_TSS);
>> }
>>
>> -static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq)
>> +static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq,
>> + bool init)
>> {
>> struct net_device *dev = tp->dev;
>> int i, err;
>> @@ -10443,6 +10500,12 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq)
>> tg3_flag_set(tp, INIT_COMPLETE);
>> tg3_enable_ints(tp);
>>
>> + if (init)
>> + tg3_ptp_init(tp);
>> + else
>> + tg3_ptp_resume(tp);
>> +
>> +
>> tg3_full_unlock(tp);
>>
>> netif_tx_start_all_queues(dev);
>> @@ -10540,11 +10603,19 @@ static int tg3_open(struct net_device *dev)
>>
>> tg3_full_unlock(tp);
>>
>> - err = tg3_start(tp, true, true);
>> + err = tg3_start(tp, true, true, true);
>> if (err) {
>> tg3_frob_aux_power(tp, false);
>> pci_set_power_state(tp->pdev, PCI_D3hot);
>> }
>> +
>> + if (tg3_flag(tp, PTP_CAPABLE)) {
>> + tp->ptp_clock = ptp_clock_register(&tp->ptp_info,
>> + &tp->pdev->dev);
>> + if (IS_ERR(tp->ptp_clock))
>> + tp->ptp_clock = NULL;
>> + }
>> +
>> return err;
>> }
>>
>> @@ -10552,6 +10623,8 @@ static int tg3_close(struct net_device *dev)
>> {
>> struct tg3 *tp = netdev_priv(dev);
>>
>> + tg3_ptp_fini(tp);
>> +
>> tg3_stop(tp);
>>
>> /* Clear stats across close / open calls */
>> @@ -11454,7 +11527,7 @@ static int tg3_set_channels(struct net_device *dev,
>>
>> tg3_carrier_off(tp);
>>
>> - tg3_start(tp, true, false);
>> + tg3_start(tp, true, false, false);
>>
>> return 0;
>> }
>> @@ -12507,7 +12580,6 @@ static void tg3_self_test(struct net_device *dev, struct ethtool_test *etest,
>> }
>>
>> tg3_full_lock(tp, irq_sync);
>> -
>> tg3_halt(tp, RESET_KIND_SUSPEND, 1);
>> err = tg3_nvram_lock(tp);
>> tg3_halt_cpu(tp, RX_CPU_BASE);
>> @@ -16598,8 +16670,8 @@ static void tg3_io_resume(struct pci_dev *pdev)
>> tg3_full_lock(tp, 0);
>> tg3_flag_set(tp, INIT_COMPLETE);
>> err = tg3_restart_hw(tp, 1);
>> - tg3_full_unlock(tp);
>> if (err) {
>> + tg3_full_unlock(tp);
> This is hunk or the next one somehow related to the PTP code?
> If not, they it should go into their own patch.
>
Yes, they are related. tg3_netif_start() now calls tg3_ptp_resume() to
reinitialize the hwclock after a chip reset. This should be inside a
full_lock(). This hunk moves the tg3_full_unlock() below tg3_netif_start(). It
also brings netif_device_attach() and tg3_timer_start() inside the lock() but
this seems to be ok since other places already do that.
I will fix the other comments in v2.
>> netdev_err(netdev, "Cannot restart hardware after reset.\n");
>> goto done;
>> }
>> @@ -16610,6 +16682,8 @@ static void tg3_io_resume(struct pci_dev *pdev)
>>
>> tg3_netif_start(tp);
>>
>> + tg3_full_unlock(tp);
>> +
>> tg3_phy_start(tp);
>>
>> done:
> Thanks,
> Richard
>
^ permalink raw reply
* Re: [PATCH] net: ICMPv6 packets transmitted on wrong interface if nfmark is mangled
From: Dries De Winter @ 2012-12-03 21:31 UTC (permalink / raw)
To: David Miller; +Cc: pablo, kaber, netdev, netfilter-devel
In-Reply-To: <20121203.141128.206409637987621093.davem@davemloft.net>
2012/12/3 David Miller <davem@davemloft.net>:
> Thinking about this some more I can't see how this is correct.
>
> What if netfilter modified one of the keys that go into the route
> lookup such as the source or destination address?
That is a question I have as well. What if the destination address of
a neighbour solicitation is rewritten to some random unicast address
for example? You could say that in that case indeed the routing table
should be followed. But you could also say that ICMPv6 is a
fundamental part of IPv6 and sending out a neighbour solicitation for
instance on a different interface than the one it is intended for is
wrong. Or you could even say that it is a total non-issue because
rewriting the destination address of ICMPv6 is already wrong in the
first place.
Anyway, what if the source address is modified while there is no
source based routing or skb->mark is modified while there is no policy
based routing? In that case routing is not affected but still the
ICMPv6 packet will go out on a different interface than the one you
would expect. This is because the dst of such packet is special in the
sense that it is not referred to by the routing table, so when
rerouting the packet it is impossible to find back the original
destination.
Not fixing this means that skb->mark is unavailable for use on ICMPv6
packets because it will inevitably put those packets on the wrong
interface. I use skb->mark for QoS, not for routing so I don't expect
the outgoing interface to be affected by my markers. Now that I know
this issue, it is easy enough for me to work around, but I suspect
that I'm not the only one in the world that uses skb->mark for other
purposes than routing. Moreover, the road from seeing a neighbour
solicitation or MLD report going out on the wrong interface to finding
this limitation can be quite painful. Anyway, in the end it's up to
you to decide of course.
Kind regards,
Dries.
^ permalink raw reply
* Re: [RFT PATCH] 8139cp: properly support change of MTU values [v2]
From: Francois Romieu @ 2012-12-03 20:46 UTC (permalink / raw)
To: jogreene; +Cc: David Miller, netdev, dwmw2
In-Reply-To: <20121203.135200.1242505353532930826.davem@davemloft.net>
David Miller <davem@davemloft.net> :
[...]
> I've applied this to net-next, if it triggers any problems we have
> some time to work it out before 3.8 is released.
I have bounced the messages to David Woodhouse since he authored the
last 8139cp changes in net-next and owns the hardware to notice
regressions.
My message of two days ago was wrong : it is not possible for the irq
handler to process a Tx event after the rings have been freed. Things
still look racy wrt netpoll though.
Any objection against the patch below ?
(I did not gotoize the dev == NULL test: it is really unlikely and
should go away).
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 0da3f5e..57cd542 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -577,28 +577,30 @@ static irqreturn_t cp_interrupt (int irq, void *dev_instance)
{
struct net_device *dev = dev_instance;
struct cp_private *cp;
+ int handled = 0;
u16 status;
if (unlikely(dev == NULL))
return IRQ_NONE;
cp = netdev_priv(dev);
+ spin_lock(&cp->lock);
+
status = cpr16(IntrStatus);
if (!status || (status == 0xFFFF))
- return IRQ_NONE;
+ goto out_unlock;
+
+ handled = 1;
netif_dbg(cp, intr, dev, "intr, status %04x cmd %02x cpcmd %04x\n",
status, cpr8(Cmd), cpr16(CpCmd));
cpw16(IntrStatus, status & ~cp_rx_intr_mask);
- spin_lock(&cp->lock);
-
/* close possible race's with dev_close */
if (unlikely(!netif_running(dev))) {
cpw16(IntrMask, 0);
- spin_unlock(&cp->lock);
- return IRQ_HANDLED;
+ goto out_unlock;
}
if (status & (RxOK | RxErr | RxEmpty | RxFIFOOvr))
@@ -612,8 +614,6 @@ static irqreturn_t cp_interrupt (int irq, void *dev_instance)
if (status & LinkChg)
mii_check_media(&cp->mii_if, netif_msg_link(cp), false);
- spin_unlock(&cp->lock);
-
if (status & PciErr) {
u16 pci_status;
@@ -625,7 +625,10 @@ static irqreturn_t cp_interrupt (int irq, void *dev_instance)
/* TODO: reset hardware */
}
- return IRQ_HANDLED;
+out_unlock:
+ spin_unlock(&cp->lock);
+
+ return IRQ_RETVAL(handled);
}
#ifdef CONFIG_NET_POLL_CONTROLLER
^ permalink raw reply related
* Re: [PATCH] ixgbe: Use is_valid_ether_addr
From: Jeff Kirsher @ 2012-12-03 21:04 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel
In-Reply-To: <1354556831.3524.0.camel@joe-AO722>
[-- Attachment #1: Type: text/plain, Size: 6797 bytes --]
On Mon, 2012-12-03 at 09:47 -0800, Joe Perches wrote:
> On Sat, 2012-10-20 at 09:22 -0700, Joe Perches wrote:
> > Use the normal kernel test instead of a module specific one.
>
> ping?
Your timely is perfect, I just cleared out all the ixgbe patches in my
queue that were before yours, and your patch (along with several others)
are finishing up validation. So I should be pushing your patch along
with other ixgbe patches this week sometime, as long as Dave's net-next
remains open long enough for me to push. :-)
>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> > found when doing that larger style conversion,
> > might as well submit it.
> >
> > drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c | 2 +-
> > drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 27 +------------------------
> > drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 1 -
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
> > drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 9 ---------
> > drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c | 2 +-
> > 6 files changed, 4 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
> > index 1077cb2..89fe00d 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
> > @@ -1022,7 +1022,7 @@ mac_reset_top:
> > hw->mac.ops.get_san_mac_addr(hw, hw->mac.san_addr);
> >
> > /* Add the SAN MAC address to the RAR only if it's a valid address */
> > - if (ixgbe_validate_mac_addr(hw->mac.san_addr) == 0) {
> > + if (is_valid_ether_addr(hw->mac.san_addr)) {
> > hw->mac.ops.set_rar(hw, hw->mac.num_rar_entries - 1,
> > hw->mac.san_addr, 0, IXGBE_RAH_AV);
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > index dbf37e4..2d8f76d 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > @@ -1762,30 +1762,6 @@ s32 ixgbe_update_eeprom_checksum_generic(struct ixgbe_hw *hw)
> > }
> >
> > /**
> > - * ixgbe_validate_mac_addr - Validate MAC address
> > - * @mac_addr: pointer to MAC address.
> > - *
> > - * Tests a MAC address to ensure it is a valid Individual Address
> > - **/
> > -s32 ixgbe_validate_mac_addr(u8 *mac_addr)
> > -{
> > - s32 status = 0;
> > -
> > - /* Make sure it is not a multicast address */
> > - if (IXGBE_IS_MULTICAST(mac_addr))
> > - status = IXGBE_ERR_INVALID_MAC_ADDR;
> > - /* Not a broadcast address */
> > - else if (IXGBE_IS_BROADCAST(mac_addr))
> > - status = IXGBE_ERR_INVALID_MAC_ADDR;
> > - /* Reject the zero address */
> > - else if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > - mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> > - status = IXGBE_ERR_INVALID_MAC_ADDR;
> > -
> > - return status;
> > -}
> > -
> > -/**
> > * ixgbe_set_rar_generic - Set Rx address register
> > * @hw: pointer to hardware structure
> > * @index: Receive address register to write
> > @@ -1889,8 +1865,7 @@ s32 ixgbe_init_rx_addrs_generic(struct ixgbe_hw *hw)
> > * to the permanent address.
> > * Otherwise, use the permanent address from the eeprom.
> > */
> > - if (ixgbe_validate_mac_addr(hw->mac.addr) ==
> > - IXGBE_ERR_INVALID_MAC_ADDR) {
> > + if (!is_valid_ether_addr(hw->mac.addr)) {
> > /* Get the MAC address from the RAR0 for later reference */
> > hw->mac.ops.get_mac_addr(hw, hw->mac.addr);
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > index d813d11..f49ca84 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > @@ -80,7 +80,6 @@ s32 ixgbe_enable_rx_dma_generic(struct ixgbe_hw *hw, u32 regval);
> > s32 ixgbe_fc_enable_generic(struct ixgbe_hw *hw);
> > void ixgbe_fc_autoneg(struct ixgbe_hw *hw);
> >
> > -s32 ixgbe_validate_mac_addr(u8 *mac_addr);
> > s32 ixgbe_acquire_swfw_sync(struct ixgbe_hw *hw, u16 mask);
> > void ixgbe_release_swfw_sync(struct ixgbe_hw *hw, u16 mask);
> > s32 ixgbe_get_san_mac_addr_generic(struct ixgbe_hw *hw, u8 *san_mac_addr);
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index e2a6691..3bb3485 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -7345,7 +7345,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
> > memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
> > memcpy(netdev->perm_addr, hw->mac.perm_addr, netdev->addr_len);
> >
> > - if (ixgbe_validate_mac_addr(netdev->perm_addr)) {
> > + if (!is_valid_ether_addr(netdev->perm_addr)) {
> > e_dev_err("invalid MAC address\n");
> > err = -EIO;
> > goto err_sw_init;
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > index 0722f33..9ddac64 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > @@ -1833,15 +1833,6 @@ enum {
> > /* Number of 100 microseconds we wait for PCI Express master disable */
> > #define IXGBE_PCI_MASTER_DISABLE_TIMEOUT 800
> >
> > -/* Check whether address is multicast. This is little-endian specific check.*/
> > -#define IXGBE_IS_MULTICAST(Address) \
> > - (bool)(((u8 *)(Address))[0] & ((u8)0x01))
> > -
> > -/* Check whether an address is broadcast. */
> > -#define IXGBE_IS_BROADCAST(Address) \
> > - ((((u8 *)(Address))[0] == ((u8)0xff)) && \
> > - (((u8 *)(Address))[1] == ((u8)0xff)))
> > -
> > /* RAH */
> > #define IXGBE_RAH_VIND_MASK 0x003C0000
> > #define IXGBE_RAH_VIND_SHIFT 18
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> > index de4da52..c73b929 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x540.c
> > @@ -152,7 +152,7 @@ mac_reset_top:
> > hw->mac.ops.get_san_mac_addr(hw, hw->mac.san_addr);
> >
> > /* Add the SAN MAC address to the RAR only if it's a valid address */
> > - if (ixgbe_validate_mac_addr(hw->mac.san_addr) == 0) {
> > + if (is_valid_ether_addr(hw->mac.san_addr)) {
> > hw->mac.ops.set_rar(hw, hw->mac.num_rar_entries - 1,
> > hw->mac.san_addr, 0, IXGBE_RAH_AV);
> >
> >
>
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Re: Re: [B.A.T.M.A.N.] [PATCH] net: fix possible deadlocks in rtnl_trylock/unlock
From: Sven Eckelmann @ 2012-12-03 20:50 UTC (permalink / raw)
To: Antonio Quartulli
Cc: Eric Dumazet, Simon Wunderlich, b.a.t.m.a.n, netdev, davem,
Simon Wunderlich
In-Reply-To: <20121203200906.GJ27828@ritirata.org>
[-- Attachment #1: Type: text/plain, Size: 954 bytes --]
On Monday 03 December 2012 21:09:06 Antonio Quartulli wrote:
> But still we have the problem in batman-adv (as Sven pointed out in a
> previous email) that tries to unregister a device in that "critical
> window".
>
> Exporting __rtnl_unlock() would solve the issue in this case.
>
> If you think the bridge code should not end up in such situation, what if
> Simon resends the patch with only the __rtnl_unlock() exportation and the
> change in batman-adv?
I personally have big doubts about the removal of the second half of the
unregister. Doesn't sound like the best idea. This would result in side
effects... one of them for example would be the possible deadlock scenario
moved to the other users of rtnl_trylock which don't unregister a device
inside their critical section.... so either you do it always this way when
using rtnl_trylock or it will break. I don't want to imagine other problems
caused by this change.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [GIT PULL] Remove __dev* markings from the networking drivers
From: David Miller @ 2012-12-03 20:36 UTC (permalink / raw)
To: gregkh; +Cc: netdev, wfp5p
In-Reply-To: <20121203202743.GA29451@kroah.com>
From: Greg KH <gregkh@linuxfoundation.org>
Date: Mon, 3 Dec 2012 12:27:43 -0800
> Here's a pull request for all of Bill's __dev* removal patches for the
> networking drivers, based on net-next. I'll send out a separate one for
> the wireless drivers in case those want to go through John's tree
>
> If I should rework these in any way, please let me know.
Pulled, thanks Greg.
^ permalink raw reply
* Re: [PATCH v2] ipv6: Fix default route failover when CONFIG_IPV6_ROUTER_PREF=n
From: David Miller @ 2012-12-03 20:35 UTC (permalink / raw)
To: pmarks; +Cc: yoshfuji, netdev
In-Reply-To: <1354566414-5752-1-git-send-email-pmarks@google.com>
From: Paul Marks <pmarks@google.com>
Date: Mon, 3 Dec 2012 12:26:54 -0800
> I believe this commit from 2008 was incorrect:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=398bcbebb6f721ac308df1e3d658c0029bb74503
>
> When CONFIG_IPV6_ROUTER_PREF is disabled, the kernel should follow
> RFC4861 section 6.3.6: if no route is NUD_VALID, then traffic should be
> sprayed across all routers (indirectly triggering NUD) until one of them
> becomes NUD_VALID.
>
> However, the following experiment demonstrates that this does not work:
>
> 1) Connect to an IPv6 network.
> 2) Change the router's MAC (and link-local) address.
>
> The kernel will lock onto the first router and never try the new one, even
> if the first becomes unreachable. This patch fixes the problem by
> allowing rt6_check_neigh() to return 0; if all routers return 0, then
> rt6_select() will fall back to round-robin behavior.
>
> This patch should have no effect when CONFIG_IPV6_ROUTER_PREF=y.
>
> Note that rt6_check_neigh() is only used in a boolean context, so I've
> changed its return type accordingly.
>
> Signed-off-by: Paul Marks <pmarks@google.com>
Applied to net-next, thanks Paul.
^ permalink raw reply
* [GIT PULL] Remove __dev* markings from the networking drivers
From: Greg KH @ 2012-12-03 20:29 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA, John W. Linville
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Bill Pemberton
Hi John,
Here's a pull request for all of Bill's __dev* removal patches for the
wireless networking drivers, based on net-next. If you want this
rebased on a different tree, or sent as patches (it's only 18 of them),
please let me know.
thanks,
greg k-h
-------------
The following changes since commit 60e476d02129acb1f863a9b4932358678ee6a355:
bna: remove useless calls to memset(). (2012-12-02 20:32:56 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/net-next.git tags/dev_removal_wireless
for you to fetch changes up to 40ccc6ab03053a2ad1f52c8b3f4d3ee828b7c013:
rtlwifi: remove __dev* attributes (2012-12-03 11:47:38 -0800)
----------------------------------------------------------------
Wireless: Remove __dev* markings from the wireless drivers
This is a series of patches that remove the dev* attributes for all
wireless drivers.
Use of __devinit, __devexit_p, __devinitdata, __devinitconst, and
__devexit are no longer needed since CONFIG_HOTPLUG is being removed as
an option.
Note, there are some devinit compiler section mismatch warnings due to
this series, but they are fixed up when merged with my driver-next
branch, which fixes the PCI device id warnings, and removes the modpost
detection, as it's no longer needed.
Signed-off-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
----------------------------------------------------------------
Bill Pemberton (18):
rfkill: remove __dev* attributes
wireless: remove __dev* attributes
ath5k: remove __dev* attributes
atmel: remove __dev* attributes
b43: remove __dev* attributes
brcm80211: remove __dev* attributes
ipw2x00: remove __dev* attributes
iwlegacy: remove __dev* attributes
iwlwifi: remove __dev* attributes
libertas: remove __dev* attributes
mwl8k: remove __dev* attributes
orinoco: remove __dev* attributes
p54: remove __dev* attributes
rt2x00: remove __dev* attributes
rtl8187: remove __dev* attributes
rtl8187: remove __dev* attributes
wlcore/wl18xx/wl12xx: remove __dev* attributes
rtlwifi: remove __dev* attributes
drivers/net/wireless/adm8211.c | 6 +++---
drivers/net/wireless/airo.c | 6 +++---
drivers/net/wireless/ath/ath5k/base.c | 4 ++--
drivers/net/wireless/ath/ath5k/led.c | 2 +-
drivers/net/wireless/ath/ath5k/pci.c | 6 +++---
drivers/net/wireless/atmel_pci.c | 6 +++---
drivers/net/wireless/b43/pcmcia.c | 6 +++---
drivers/net/wireless/b43/sdio.c | 6 +++---
drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c | 4 ++--
drivers/net/wireless/ipw2x00/ipw2100.c | 4 ++--
drivers/net/wireless/ipw2x00/ipw2200.c | 8 ++++----
drivers/net/wireless/iwlegacy/3945-mac.c | 4 ++--
drivers/net/wireless/iwlegacy/4965-mac.c | 4 ++--
drivers/net/wireless/iwlwifi/pcie/drv.c | 4 ++--
drivers/net/wireless/libertas/if_spi.c | 6 +++---
drivers/net/wireless/mwl8k.c | 12 ++++++------
drivers/net/wireless/orinoco/orinoco_nortel.c | 4 ++--
drivers/net/wireless/orinoco/orinoco_pci.c | 4 ++--
drivers/net/wireless/orinoco/orinoco_plx.c | 4 ++--
drivers/net/wireless/orinoco/orinoco_tmd.c | 4 ++--
drivers/net/wireless/p54/p54pci.c | 6 +++---
drivers/net/wireless/p54/p54spi.c | 6 +++---
drivers/net/wireless/p54/p54usb.c | 6 +++---
drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2800pci.c | 4 ++--
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
drivers/net/wireless/rtl818x/rtl8180/dev.c | 6 +++---
drivers/net/wireless/rtl818x/rtl8187/dev.c | 6 +++---
drivers/net/wireless/rtlwifi/pci.c | 2 +-
drivers/net/wireless/rtlwifi/pci.h | 2 +-
drivers/net/wireless/rtlwifi/rtl8192de/sw.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192se/sw.c | 2 +-
drivers/net/wireless/rtlwifi/usb.c | 2 +-
drivers/net/wireless/rtlwifi/usb.h | 2 +-
drivers/net/wireless/ti/wl1251/sdio.c | 4 ++--
drivers/net/wireless/ti/wl1251/spi.c | 6 +++---
drivers/net/wireless/ti/wl12xx/main.c | 6 +++---
drivers/net/wireless/ti/wl18xx/main.c | 6 +++---
drivers/net/wireless/ti/wlcore/main.c | 4 ++--
drivers/net/wireless/ti/wlcore/sdio.c | 8 ++++----
drivers/net/wireless/ti/wlcore/spi.c | 6 +++---
drivers/net/wireless/ti/wlcore/wlcore.h | 4 ++--
net/rfkill/rfkill-gpio.c | 2 +-
net/rfkill/rfkill-regulator.c | 6 +++---
45 files changed, 104 insertions(+), 104 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox