* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Eric W. Biederman @ 2013-09-14 1:32 UTC (permalink / raw)
To: David Miller; +Cc: fruggeri, edumazet, jiri, alexander.h.duyck, amwang, netdev
In-Reply-To: <20130913.201626.1381750756413200302.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 12 Sep 2013 13:06:53 -0700
>
>> David, Eric, do any of you know why we have this netdevice notfier
>> rebroadcast?
>
> It's been there as long as I can remember, but I don't remember
> exactly why we do that.
>
> If I had to guess, it's to handle something that has to retry later
> due to a "spin_trylock()" or similar type of situation.
>
> Probably best to not remove it. :)
Since it doesn't completely solve the problem, it isn't worth it to
solve this problem. Sigh
I have added this bit of code and the moving of references to the
loopback device in dst_ifdown to my hitlist of things to sort out some
day when I have plenty time.
Eric
^ permalink raw reply
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Eric W. Biederman @ 2013-09-14 1:46 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <CA+HUmGjEC8eXT7BHm20uVOagJQbsqO9VZjmGSgmUvnrtEH-JYw@mail.gmail.com>
Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
> On Thu, Sep 12, 2013 at 10:50 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
> To summarize your proposal:
> 1) Remove re-broadcasting of NETDEV_UNREGISTER and
> NETDEV_UNREGISTER_FINAL from netdev_wait_allrefs.
Forget that it isn't needed. It would be nice but since it doesn't
solve the entire problem let's not go there.
> 2) If unregistering a net_device causes unregistering of other virtual
> devices (eg veth, macvlan, vlan) then move the virtual devices to the
> namespace of the original net_device.
>
> I do have some concerns about both correctness and feasibility of this approach.
>
> About 2), namespace dependent operations triggered by unregistering
> the virtual devices (eg rt_flush_dev, dst_dev_event/dst_ifdown and
> probably more) would not be done in the namespaces where they should.
Yes they will. That is what dev_change_net_namespace does.
dev_change_net_namespace would not be correct if it did not do that.
Now dev_change_net_namespace is comparitively expensive so it may not be the
best approach but it is an approach that will work.
> I do urge you to take a second look at the approach that I proposed.
> All it does is make sure that a namespace loopback_dev is not removed
> until any devices unlisted from that namespace are also disposed of.
> That in turn prevents non-device pernet subsystems from exiting that
> namespace in ops_exit_list.
It was worth a second look. I can not find anything wrong with your
patch but I can not convince myself that it is correct either. The
moving around the loopback device in the net dev todo list to prevent
deadlock I can't imagine why you are doing that.
I think I would prefer something more explicit and less likely to break.
Perhaps something that keeps a network namespace from exiting while we
delete devices. Using the loopback device like that looks fragile.
However it is very true that we require the loopback device to stay
around until all of the dst_ifdown calls in a network namespace are
made.
At least my original concern that you could be incrementing the count on
the loop back device when it had already reached zero can't be the case.
It would be very nice to have something that I could think through
easily and was robust to change.
Eric
^ permalink raw reply
* [PATCH] ptp: measure the time offset between PHC and system clock
From: Dong Zhu @ 2013-09-14 8:03 UTC (permalink / raw)
To: Richard Cochran, Rob Landley; +Cc: netdev, linux-doc, linux-kernel
This patch add a method into testptp.c to measure the time offset
between phc and system clock through the ioctl PTP_SYS_OFFSET.
Signed-off-by: Dong Zhu <bluezhudong@gmail.com>
---
Documentation/ptp/testptp.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
index f59ded0..72bb030 100644
--- a/Documentation/ptp/testptp.c
+++ b/Documentation/ptp/testptp.c
@@ -112,6 +112,7 @@ static void usage(char *progname)
" -f val adjust the ptp clock frequency by 'val' ppb\n"
" -g get the ptp clock time\n"
" -h prints this message\n"
+ " -k val measure the time offset between PHC and system clock\n"
" -p val enable output with a period of 'val' nanoseconds\n"
" -P val enable or disable (val=1|0) the system clock PPS\n"
" -s set the ptp clock time from the system time\n"
@@ -133,8 +134,12 @@ int main(int argc, char *argv[])
struct itimerspec timeout;
struct sigevent sigevent;
+ struct ptp_clock_time *pct;
+ struct ptp_sys_offset *sysoff;
+
+
char *progname;
- int c, cnt, fd;
+ int i, c, cnt, fd;
char *device = DEVICE;
clockid_t clkid;
@@ -144,6 +149,8 @@ int main(int argc, char *argv[])
int extts = 0;
int gettime = 0;
int oneshot = 0;
+ int offset = 0;
+ int n_samples = 0;
int periodic = 0;
int perout = -1;
int pps = -1;
@@ -151,7 +158,7 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
- while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) {
+ while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) {
switch (c) {
case 'a':
oneshot = atoi(optarg);
@@ -174,6 +181,10 @@ int main(int argc, char *argv[])
case 'g':
gettime = 1;
break;
+ case 'k':
+ offset = 1;
+ n_samples = atoi(optarg);
+ break;
case 'p':
perout = atoi(optarg);
break;
@@ -376,6 +387,31 @@ int main(int argc, char *argv[])
}
}
+ if (offset) {
+ sysoff = calloc(1, sizeof(*sysoff));
+ if (!sysoff) {
+ perror("calloc");
+ return -1;
+ }
+ sysoff->n_samples = n_samples;
+
+ if (ioctl(fd, PTP_SYS_OFFSET, sysoff))
+ perror("PTP_SYS_OFFSET");
+ else
+ puts("time offset between PHC and
+ system clock request okay");
+
+ pct = &sysoff->ts[0];
+ for (i = 0; i < sysoff->n_samples; i++, pct++) {
+ printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
+ pct++;
+ printf("phc time: %ld.%ld\n\n", pct->sec, pct->nsec);
+ }
+ printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
+
+ free(sysoff);
+ }
+
close(fd);
return 0;
}
--
1.7.11.7
--
Best Regards,
Dong Zhu
^ permalink raw reply related
* Re: [PATCH v2 1/6] ipv6: del the statements for updating route in (dccp|tcp|sctp)_v6_err
From: Daniel Borkmann @ 2013-09-14 8:20 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Duan Jiong, davem, netdev, vyasevic, linux-sctp
In-Reply-To: <20130913203721.GB32431@order.stressinduktion.org>
> [added linux-sctp, Daniel and Vlad + full quote]
>
> On Fri, Sep 13, 2013 at 10:58:55AM +0800, Duan Jiong wrote:
> > From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> >
> > Because we will do route updating for redirect in nidsc layer. And
> > when dealing with redirect message, the dccp and sctp should like
> > tcp return directly.
> >
> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
[...]
> Please Cc linux-sctp@vger.kernel.org on the sctp bits in the next round,
> too.
>
> Otherwise I looked at all patches and they seemed fine to me.
>
> Duan, I would suggest the following:
>
> I cannot judge if the patch Daniel proposed regarding EPROTO in sk->sk_err
> on redirects should go to stable. If it should, perhaps this patch should
> go in first and you could later on rebase this series as soon as Daniel's
> patch has landed in the net repo? Only some minor edits should be needed
> then. This way there is a clean patch for stable and David can consider
> taking this in for net or net-next (this fixes a glitch where we do not
> apply redirects generated by packets of ipv6 tunnels, otherwise a bit of
> code removal).
Thanks Hannes !
I'm on travel over the weekend and mostly offline, so I'll look into this
on right on Monday morning.
Cheers and thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net 0/3] SLCAN/SLIP fixes and performance
From: Marc Kleine-Budde @ 2013-09-14 10:45 UTC (permalink / raw)
To: Andre Naujoks; +Cc: davem, linux-can, netdev, linux-kernel
In-Reply-To: <1379093833-4949-1-git-send-email-nautsch2@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 988 bytes --]
On 09/13/2013 07:37 PM, Andre Naujoks wrote:
> Hi Dave,
>
> these are some loosely related patches, that fix an ancient locking problem in
> the slip and slcan drivers, add general ASCII-HEX to bin functions for
> uppercase ASCII, fix the handling of CAN RTR frames in the slcan driver
Can you get an Acked-by for the ASCII-HEX functions from the appropriate
maintainer?
> and increase the performance for the slcan driver.
>
> As these patches mainly contain fixes for the slip/slcan drivers that require
> a tty layer fix included in 3.11, I would suggest to get the patches in via
> the net tree for the 3.12 cycle. They should apply properly on the latest net
> and mainline tree.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [PATCH net 0/3] SLCAN/SLIP fixes and performance
From: Andre Naujoks @ 2013-09-14 11:22 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: davem, linux-can, netdev, linux-kernel
In-Reply-To: <52343E64.1050207@pengutronix.de>
On 14.09.2013 12:45, Marc Kleine-Budde wrote:
> On 09/13/2013 07:37 PM, Andre Naujoks wrote:
>> Hi Dave,
>>
>> these are some loosely related patches, that fix an ancient locking problem in
>> the slip and slcan drivers, add general ASCII-HEX to bin functions for
>> uppercase ASCII, fix the handling of CAN RTR frames in the slcan driver
>
> Can you get an Acked-by for the ASCII-HEX functions from the appropriate
> maintainer?
The patch went out to the maintainers I got from the get_maintainer.pl
script. Is there anything else I can or should do to get an Ack from them?
Regards
Andre
>
>> and increase the performance for the slcan driver.
>>
>> As these patches mainly contain fixes for the slip/slcan drivers that require
>> a tty layer fix included in 3.11, I would suggest to get the patches in via
>> the net tree for the 3.12 cycle. They should apply properly on the latest net
>> and mainline tree.
>
> Marc
>
^ permalink raw reply
* [PATCH] Do not drop DNATed 6to4/6rd packets
From: Catalin(ux) M. BOIE @ 2013-09-14 10:30 UTC (permalink / raw)
To: netdev; +Cc: hannes, yoshfuji, davem
From: "Catalin(ux) M. BOIE" <catab@embedromix.ro>
When a router is doing DNAT for 6to4/6rd packets the latest anti-spoofing
patch (218774dc) will drop them because the IPv6 address embedded
does not match the IPv4 destination. This patch will allow them to
pass by testing if we have an address that matches on 6to4/6rd interface.
I have been hit by this problem using Fedora and IPV6TO4_IPV4ADDR.
Also, log the dropped packets (with rate limit).
Signed-off-by: Catalin(ux) M. BOIE <catab@embedromix.ro>
---
include/net/addrconf.h | 5 +++++
net/ipv6/addrconf.c | 28 ++++++++++++++++++++++++++++
net/ipv6/sit.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 84a6440..49886fc 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -73,6 +73,11 @@ extern int ipv6_chk_home_addr(struct net *net,
const struct in6_addr *addr);
#endif
+extern bool ipv6_chk_custom_prefix(
+ const struct in6_addr *addr,
+ const unsigned int prefix_len,
+ struct net_device *dev);
+
extern int ipv6_chk_prefix(const struct in6_addr *addr,
struct net_device *dev);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d1ab6ab..70ab1ba 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1527,6 +1527,34 @@ static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
return false;
}
+/*
+ * Compares an address/prefix_len with addresses on device @dev.
+ * If one is found it returns true.
+ */
+bool ipv6_chk_custom_prefix(const struct in6_addr *addr,
+ const unsigned int prefix_len, struct net_device *dev)
+{
+ struct inet6_dev *idev;
+ struct inet6_ifaddr *ifa;
+ bool ret = false;
+
+ rcu_read_lock();
+ idev = __in6_dev_get(dev);
+ if (idev) {
+ read_lock_bh(&idev->lock);
+ list_for_each_entry(ifa, &idev->addr_list, if_list) {
+ ret = ipv6_prefix_equal(addr, &ifa->addr, prefix_len);
+ if (ret)
+ break;
+ }
+ read_unlock_bh(&idev->lock);
+ }
+ rcu_read_unlock();
+
+ return ret;
+}
+EXPORT_SYMBOL(ipv6_chk_custom_prefix);
+
int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev)
{
struct inet6_dev *idev;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 3353634..fd2aba9 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -566,6 +566,33 @@ static inline bool is_spoofed_6rd(struct ip_tunnel *tunnel, const __be32 v4addr,
return false;
}
+/*
+ * Checks if an address matches an address on the tunnel interface.
+ * Used to detect the NAT of proto 41 packets and let them pass spoofing test.
+ * Long story:
+ * This function is called after we considered the packet as spoofed
+ * in is_spoofed_6rd.
+ * We may have a router that is doing NAT for proto 41 packets
+ * for an internal station. Destination a.a.a.a/PREFIX:bbbb:bbbb
+ * will be translated to n.n.n.n/PREFIX:bbbb:bbbb. And is_spoofed_6rd
+ * function will return true, dropping the packet.
+ * But, we can still check if is spoofed against the IP
+ * addresses associated with the interface.
+ */
+static bool only_dnatted(const struct ip_tunnel *tunnel,
+ const struct in6_addr *v6dst)
+{
+ int prefix_len;
+
+#ifdef CONFIG_IPV6_SIT_6RD
+ prefix_len = tunnel->ip6rd.prefixlen + 32
+ - tunnel->ip6rd.relay_prefixlen;
+#else
+ prefix_len = 48;
+#endif
+ return ipv6_chk_custom_prefix(v6dst, prefix_len, tunnel->dev);
+}
+
static int ipip6_rcv(struct sk_buff *skb)
{
const struct iphdr *iph = ip_hdr(skb);
@@ -589,11 +616,20 @@ static int ipip6_rcv(struct sk_buff *skb)
tunnel->dev->stats.rx_errors++;
goto out;
}
- } else {
- if (is_spoofed_6rd(tunnel, iph->saddr,
- &ipv6_hdr(skb)->saddr) ||
- is_spoofed_6rd(tunnel, iph->daddr,
- &ipv6_hdr(skb)->daddr)) {
+ } else if (!(tunnel->dev->flags&IFF_POINTOPOINT)) {
+ if (unlikely(is_spoofed_6rd(tunnel, iph->saddr,
+ &ipv6_hdr(skb)->saddr))) {
+ net_warn_ratelimited("Src spoofed %pI4/%pI6c\n",
+ &iph->saddr, &ipv6_hdr(skb)->saddr);
+ tunnel->dev->stats.rx_errors++;
+ goto out;
+ }
+
+ if (unlikely(is_spoofed_6rd(tunnel, iph->daddr,
+ &ipv6_hdr(skb)->daddr)
+ && !only_dnatted(tunnel, &ipv6_hdr(skb)->daddr))) {
+ net_warn_ratelimited("Dst spoofed %pI4/%pI6c\n",
+ &iph->daddr, &ipv6_hdr(skb)->daddr);
tunnel->dev->stats.rx_errors++;
goto out;
}
@@ -713,7 +749,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
neigh = dst_neigh_lookup(skb_dst(skb), &iph6->daddr);
if (neigh == NULL) {
- net_dbg_ratelimited("sit: nexthop == NULL\n");
+ net_dbg_ratelimited("nexthop == NULL\n");
goto tx_error;
}
@@ -742,7 +778,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
neigh = dst_neigh_lookup(skb_dst(skb), &iph6->daddr);
if (neigh == NULL) {
- net_dbg_ratelimited("sit: nexthop == NULL\n");
+ net_dbg_ratelimited("nexthop == NULL\n");
goto tx_error;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH 2/4] net: Add lower dev list helpers
From: Veaceslav Falico @ 2013-09-14 12:27 UTC (permalink / raw)
To: John Fastabend
Cc: stephen, bhutchings, ogerlitz, john.ronciak, netdev,
shannon.nelson
In-Reply-To: <20130911184647.26914.40440.stgit@nitbit.x32>
On Wed, Sep 11, 2013 at 11:46:49AM -0700, John Fastabend wrote:
>This patch adds helpers to traverse the lower dev lists, these
>helpers match the upper dev list implementation.
>
>VSI implementers may use these to track a list of connected netdevs.
>This is easier then having drivers do their own accounting.
Just as a note (as I have quite no idea how ixgbe works) - you are aware
that the upper/lower lists currently include *all* upper/lower devices, and
not only the first-connected ones?
I've seen that you usually verify it, however not always, just a heads-up -
sorry if misread.
I'm also currently trying to get the new patchset included - which would
split the first-tier connected devices from all the 'other' (as in - lower
of a lower and upper of an upper) devices, so that way, I think, it would
be easier/faster for you to use it. It also has a ->private field, easily
accessible, which you could have used instead of/in conjunction with/for
struct ixgbe_vsi_adapter.
[PATCH v2 net-next 00/27] bonding: use neighbours instead of own lists
Here's the patchset.
Hope that helps.
>
>Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>---
> include/linux/netdevice.h | 8 ++++++++
> net/core/dev.c | 25 +++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 041b42a..4d24b38 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -2813,6 +2813,8 @@ extern int bpf_jit_enable;
> extern bool netdev_has_upper_dev(struct net_device *dev,
> struct net_device *upper_dev);
> extern bool netdev_has_any_upper_dev(struct net_device *dev);
>+extern struct net_device *netdev_lower_get_next_dev_rcu(struct net_device *dev,
>+ struct list_head **iter);
> extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
> struct list_head **iter);
>
>@@ -2823,6 +2825,12 @@ extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
> upper; \
> upper = netdev_upper_get_next_dev_rcu(dev, &(iter)))
>
>+#define netdev_for_each_lower_dev_rcu(dev, lower, iter) \
>+ for (iter = &(dev)->lower_dev_list, \
>+ lower = netdev_lower_get_next_dev_rcu(dev, &(iter)); \
>+ lower; \
>+ lower = netdev_lower_get_next_dev_rcu(dev, &(iter)))
>+
> extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
> extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
> extern int netdev_upper_dev_link(struct net_device *dev,
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 5c713f2..65ed610 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4468,6 +4468,31 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
> }
> EXPORT_SYMBOL(netdev_master_upper_dev_get);
>
>+/* netdev_lower_get_next_dev_rcu - Get the next dev from lower list
>+ * @dev: device
>+ * @iter: list_head ** of the current position
>+ *
>+ * Gets the next device from the dev's lower list, starting from iter
>+ * position. The caller must hold RTNL/RCU read lock.
>+ */
>+struct net_device *netdev_lower_get_next_dev_rcu(struct net_device *dev,
>+ struct list_head **iter)
>+{
>+ struct netdev_adjacent *lower;
>+
>+ WARN_ON_ONCE(!rcu_read_lock_held());
>+
>+ lower = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
>+
>+ if (&lower->list == &dev->lower_dev_list)
>+ return NULL;
>+
>+ *iter = &lower->list;
>+
>+ return lower->dev;
>+}
>+EXPORT_SYMBOL(netdev_lower_get_next_dev_rcu);
>+
> /* netdev_upper_get_next_dev_rcu - Get the next dev from upper list
> * @dev: device
> * @iter: list_head ** of the current position
>
^ permalink raw reply
* Re: [PATCH] ptp: measure the time offset between PHC and system clock
From: Richard Cochran @ 2013-09-14 14:31 UTC (permalink / raw)
To: Dong Zhu; +Cc: Rob Landley, netdev, linux-doc, linux-kernel
In-Reply-To: <20130914080306.GC23682@zhudong.nay.redhat.com>
On Sat, Sep 14, 2013 at 04:03:06PM +0800, Dong Zhu wrote:
> This patch add a method into testptp.c to measure the time offset
> between phc and system clock through the ioctl PTP_SYS_OFFSET.
>
This is a nice addition to the testptp program. I do have a few
comments, below.
First off, the subject line should mention testptp. How about this?
[PATCH] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program
> Signed-off-by: Dong Zhu <bluezhudong@gmail.com>
> ---
> Documentation/ptp/testptp.c | 40 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
> index f59ded0..72bb030 100644
> --- a/Documentation/ptp/testptp.c
> +++ b/Documentation/ptp/testptp.c
> @@ -112,6 +112,7 @@ static void usage(char *progname)
> " -f val adjust the ptp clock frequency by 'val' ppb\n"
> " -g get the ptp clock time\n"
> " -h prints this message\n"
> + " -k val measure the time offset between PHC and system clock\n"
The help message should tell the user what 'val' is.
> " -p val enable output with a period of 'val' nanoseconds\n"
> " -P val enable or disable (val=1|0) the system clock PPS\n"
> " -s set the ptp clock time from the system time\n"
> @@ -133,8 +134,12 @@ int main(int argc, char *argv[])
> struct itimerspec timeout;
> struct sigevent sigevent;
>
> + struct ptp_clock_time *pct;
> + struct ptp_sys_offset *sysoff;
> +
> +
> char *progname;
> - int c, cnt, fd;
> + int i, c, cnt, fd;
>
> char *device = DEVICE;
> clockid_t clkid;
> @@ -144,6 +149,8 @@ int main(int argc, char *argv[])
> int extts = 0;
> int gettime = 0;
> int oneshot = 0;
> + int offset = 0;
> + int n_samples = 0;
> int periodic = 0;
> int perout = -1;
> int pps = -1;
> @@ -151,7 +158,7 @@ int main(int argc, char *argv[])
>
> progname = strrchr(argv[0], '/');
> progname = progname ? 1+progname : argv[0];
> - while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) {
> + while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) {
> switch (c) {
> case 'a':
> oneshot = atoi(optarg);
> @@ -174,6 +181,10 @@ int main(int argc, char *argv[])
> case 'g':
> gettime = 1;
> break;
> + case 'k':
> + offset = 1;
> + n_samples = atoi(optarg);
> + break;
> case 'p':
> perout = atoi(optarg);
> break;
> @@ -376,6 +387,31 @@ int main(int argc, char *argv[])
> }
> }
>
> + if (offset) {
> + sysoff = calloc(1, sizeof(*sysoff));
> + if (!sysoff) {
> + perror("calloc");
> + return -1;
> + }
> + sysoff->n_samples = n_samples;
> +
> + if (ioctl(fd, PTP_SYS_OFFSET, sysoff))
> + perror("PTP_SYS_OFFSET");
> + else
> + puts("time offset between PHC and
> + system clock request okay");
> +
> + pct = &sysoff->ts[0];
> + for (i = 0; i < sysoff->n_samples; i++, pct++) {
> + printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
> + pct++;
> + printf("phc time: %ld.%ld\n\n", pct->sec, pct->nsec);
^^^^
I think the output would look nicer with only one newline. After all,
each measurement is a {sys,phc,sys} triplet and not a {sys,phc} pair.
> + }
> + printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
> +
> + free(sysoff);
> + }
> +
Thanks,
Richard
^ permalink raw reply
* [PATCH net-next 0/2] bridge: fix NULL pointer deref of br_port_get_rcu
From: Hong Zhiguo @ 2013-09-14 14:42 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, vyasevic, Hong Zhiguo
From: Hong Zhiguo <zhiguohong@tencent.com>
I got an Oops on my box when br_handle_frame is called between these
2 lines of del_nbp:
dev->priv_flags &= ~IFF_BRIDGE_PORT;
/* --> br_handle_frame is called at this time */
netdev_rx_handler_unregister(dev);
In br_handle_frame the return of br_port_get_rcu(dev) is dereferenced
without check but br_port_get_rcu(dev) returns NULL if:
!(dev->priv_flags & IFF_BRIDGE_PORT)
Eric Dumazet pointed out the testing of IFF_BRIDGE_PORT is not necessary
here since we're in rcu_read_lock and we have synchronize_net() in
netdev_rx_handler_unregister. So removed the testing of
IFF_BRIDGE_PORT.
I tested the fix on my box with script doing "brctl addif" and "brctl
delif" repeatedly while a lot of broadcast frame present on the LAN.
I added msleep in del_nbp between setting of priv_flags and unregister
so it's easy to reproduce the oops without the fix.
I want to remove the NULL checks following call to br_port_get_rcu in
Br_netfilter and ebtables code. Could someone confirm that's OK?
The Oops(some lines omitted):
BUG: unable to handle kernel NULL pointer dereference at 0000000000000021
IP: [<ffffffff8150901d>] br_handle_frame+0xed/0x230
Oops: 0000 [#1] PREEMPT SMP
RIP: 0010:[<ffffffff8150901d>] [<ffffffff8150901d>] br_handle_frame+0xed/0x230
RSP: 0018:ffff880030403c10 EFLAGS: 00010286
Stack:
ffff88002c945700 ffffffff81508f30 0000000000000000 ffff88002d41e000
ffff880030403c98 ffffffff81477acb ffffffff81477821 ffff880030403c68
ffffffff81090e10 00ff88002d545c80 ffff88002c945700 ffffffff81aa50c0
Call Trace:
<IRQ>
[<ffffffff81508f30>] ? br_handle_frame_finish+0x300/0x300
[<ffffffff81477acb>] __netif_receive_skb_core+0x39b/0x880
Hong Zhiguo (2):
bridge: use br_port_get_rtnl within rtnl lock
bridge: fix NULL pointer deref of br_port_get_rcu
net/bridge/br_netlink.c | 4 ++--
net/bridge/br_private.h | 7 ++-----
2 files changed, 4 insertions(+), 7 deletions(-)
--
1.8.1.2
^ permalink raw reply
* [PATCH net-next 1/2] bridge: use br_port_get_rtnl within rtnl lock
From: Hong Zhiguo @ 2013-09-14 14:42 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, vyasevic, Hong Zhiguo
In-Reply-To: <1379169748-767-1-git-send-email-zhiguohong@tencent.com>
From: Hong Zhiguo <zhiguohong@tencent.com>
current br_port_get_rcu is problematic in bridging path
(NULL deref). Change these calls in netlink path first.
Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
---
net/bridge/br_netlink.c | 4 ++--
net/bridge/br_private.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index b9259ef..e74ddc1 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -207,7 +207,7 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct net_device *dev, u32 filter_mask)
{
int err = 0;
- struct net_bridge_port *port = br_port_get_rcu(dev);
+ struct net_bridge_port *port = br_port_get_rtnl(dev);
/* not a bridge port and */
if (!port && !(filter_mask & RTEXT_FILTER_BRVLAN))
@@ -451,7 +451,7 @@ static size_t br_get_link_af_size(const struct net_device *dev)
struct net_port_vlans *pv;
if (br_port_exists(dev))
- pv = nbp_get_vlan_info(br_port_get_rcu(dev));
+ pv = nbp_get_vlan_info(br_port_get_rtnl(dev));
else if (dev->priv_flags & IFF_EBRIDGE)
pv = br_get_vlan_info((struct net_bridge *)netdev_priv(dev));
else
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 598cb0b..49fb43e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -208,7 +208,7 @@ static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *d
return br_port_exists(dev) ? port : NULL;
}
-static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
+static inline struct net_bridge_port *br_port_get_rtnl(const struct net_device *dev)
{
return br_port_exists(dev) ?
rtnl_dereference(dev->rx_handler_data) : NULL;
--
1.8.1.2
^ permalink raw reply related
* [PATCH net-next 2/2] bridge: fix NULL pointer deref of br_port_get_rcu
From: Hong Zhiguo @ 2013-09-14 14:42 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, vyasevic, Hong Zhiguo
In-Reply-To: <1379169748-767-1-git-send-email-zhiguohong@tencent.com>
From: Hong Zhiguo <zhiguohong@tencent.com>
The NULL deref happens when br_handle_frame is called between these
2 lines of del_nbp:
dev->priv_flags &= ~IFF_BRIDGE_PORT;
/* --> br_handle_frame is called at this time */
netdev_rx_handler_unregister(dev);
In br_handle_frame the return of br_port_get_rcu(dev) is dereferenced
without check but br_port_get_rcu(dev) returns NULL if:
!(dev->priv_flags & IFF_BRIDGE_PORT)
Eric Dumazet pointed out the testing of IFF_BRIDGE_PORT is not necessary
here since we're in rcu_read_lock and we have synchronize_net() in
netdev_rx_handler_unregister. So remove the testing of IFF_BRIDGE_PORT
and by the previous patch, make sure br_port_get_rcu is called in
bridging code.
Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
---
net/bridge/br_private.h | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 49fb43e..1aaca0e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -202,10 +202,7 @@ struct net_bridge_port
static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
{
- struct net_bridge_port *port =
- rcu_dereference_rtnl(dev->rx_handler_data);
-
- return br_port_exists(dev) ? port : NULL;
+ return rcu_dereference(dev->rx_handler_data);
}
static inline struct net_bridge_port *br_port_get_rtnl(const struct net_device *dev)
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program clock
From: Dong Zhu @ 2013-09-14 15:39 UTC (permalink / raw)
To: Richard Cochran; +Cc: Rob Landley, netdev, linux-doc, linux-kernel
In-Reply-To: <20130914143144.GA4206@netboy>
On Sat, Sep 14, 2013 at 04:31:46PM +0200, Richard Cochran wrote:
> On Sat, Sep 14, 2013 at 04:03:06PM +0800, Dong Zhu wrote:
> > This patch add a method into testptp.c to measure the time offset
> > between phc and system clock through the ioctl PTP_SYS_OFFSET.
> >
>
> This is a nice addition to the testptp program. I do have a few
> comments, below.
>
Thanks very much for your comments, I have modified the patch as below,
Cuold you have a look at it again ? Any comments would be appreciated.
>From 655b45785a85599d5fff5eb3b8d9b49b72f2991f Mon Sep 17 00:00:00 2001
From: Dong Zhu <bluezhudong@gmail.com>
Date: Sat, 14 Sep 2013 23:32:14 +0800
This patch add a method into testptp.c to measure the time offset
between phc and system clock through the ioctl PTP_SYS_OFFSET.
Signed-off-by: Dong Zhu <bluezhudong@gmail.com>
---
Documentation/ptp/testptp.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
index f59ded0..8acdc70 100644
--- a/Documentation/ptp/testptp.c
+++ b/Documentation/ptp/testptp.c
@@ -112,6 +112,8 @@ static void usage(char *progname)
" -f val adjust the ptp clock frequency by 'val' ppb\n"
" -g get the ptp clock time\n"
" -h prints this message\n"
+ " -k val measure the time offset between phc and system clock "
+ "for 'val' times (Maximum 25)\n"
" -p val enable output with a period of 'val' nanoseconds\n"
" -P val enable or disable (val=1|0) the system clock PPS\n"
" -s set the ptp clock time from the system time\n"
@@ -133,8 +135,12 @@ int main(int argc, char *argv[])
struct itimerspec timeout;
struct sigevent sigevent;
+ struct ptp_clock_time *pct;
+ struct ptp_sys_offset *sysoff;
+
+
char *progname;
- int c, cnt, fd;
+ int i, c, cnt, fd;
char *device = DEVICE;
clockid_t clkid;
@@ -144,6 +150,8 @@ int main(int argc, char *argv[])
int extts = 0;
int gettime = 0;
int oneshot = 0;
+ int offset = 0;
+ int n_samples = 0;
int periodic = 0;
int perout = -1;
int pps = -1;
@@ -151,7 +159,7 @@ int main(int argc, char *argv[])
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
- while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) {
+ while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) {
switch (c) {
case 'a':
oneshot = atoi(optarg);
@@ -174,6 +182,10 @@ int main(int argc, char *argv[])
case 'g':
gettime = 1;
break;
+ case 'k':
+ offset = 1;
+ n_samples = atoi(optarg);
+ break;
case 'p':
perout = atoi(optarg);
break;
@@ -376,6 +388,30 @@ int main(int argc, char *argv[])
}
}
+ if (offset) {
+ sysoff = calloc(1, sizeof(*sysoff));
+ if (!sysoff) {
+ perror("calloc");
+ return -1;
+ }
+ sysoff->n_samples = n_samples;
+
+ if (ioctl(fd, PTP_SYS_OFFSET, sysoff))
+ perror("PTP_SYS_OFFSET");
+ else
+ puts("phc and system clock time offset request okay");
+
+ pct = &sysoff->ts[0];
+ for (i = 0; i < sysoff->n_samples; i++, pct++) {
+ printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
+ pct++;
+ printf("phc time: %ld.%ld\n", pct->sec, pct->nsec);
+ }
+ printf("system time: %ld.%ld\n", pct->sec, pct->nsec);
+
+ free(sysoff);
+ }
+
close(fd);
return 0;
}
--
1.7.11.7
--
Best Regards,
Dong Zhu
^ permalink raw reply related
* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-14 15:42 UTC (permalink / raw)
To: Veaceslav Falico
Cc: David Miller, makita.toshiaki, vyasevic, netdev,
Fernando Luis Vazquez Cao, Patrick McHardy
In-Reply-To: <20130913152114.GD695@redhat.com>
On Fri, 2013-09-13 at 17:21 +0200, Veaceslav Falico wrote:
> On Fri, Sep 13, 2013 at 09:06:53PM +0900, Toshiaki Makita wrote:
> >On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>
> >> > There seem to be some undesirable behaviors related with PVID.
> >> > 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >> > to any frame regardless of whether we set it or not.
> >> > 2. FDB entries learned via frames applied PVID are registered with
> >> > VID 0 rather than VID value of PVID.
> >> > 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >> > This leads interoperational problems such as sending frames with VID
> >> > 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >> > 0 as they belong to VLAN 0, which is expected to be handled as they have
> >> > no VID according to IEEE 802.1Q.
> >> >
> >> > Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >> > is fixed, because we cannot activate PVID due to it.
> >>
> >> Please work out the issues in patch #2 with Vlad and resubmit this
> >> series.
> >>
> >> Thank you.
> >
> >I'm hovering between whether we should fix the issue by changing vlan 0
> >interface behavior in 8021q module or enabling a bridge port to sending
> >priority-tagged frames, or another better way.
>
> Take a look at how was it done for bonding - it just goes through the list
> of attached vlan devs, and doesn't care about vlan0 (which can, btw, exist
> technically). I'm not sure if that's what you're looking for, but worth a
> try.
>
> bond_arp_send_all() might be a good starting point.
Thanks a lot.
I looked over it and took it that bonding driver doesn't send any arp
probe as a priority-tagged frame even if the routing table tells that
the outgoing interface to reach the arp target is bond0.0.
If we seek to take a similar way with bonding, we might be able to
simply ignore vlan 0 interface, or incoming priority-tagged frames.
I'm, however, afraid that it may be undesirable in view of
interoperability.
Anyway, thank you again for your suggestion.
I'm going to think a bit more about the issue and rearrange the patch
set.
Thanks,
Toshiaki Makita
>
> >
> >If you could comment it, I'd appreciate it :)
> >
> >
> >BTW, I think what is discussed in patch #2 is another problem about
> >handling priority-tags, and it exists without this patch set applied.
> >It looks like that we should prepare another patch set than this to fix
> >that problem.
> >
> >Should I include patches that fix the priority-tags problem in this
> >patch set and resubmit them all together?
> >
> >
> >Thanks,
> >
> >Toshiaki Makita
> >
> >>
> >> --
> >> 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
> >
> >
> >
> >--
> >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
^ permalink raw reply
* [PATCH 1/1] isdn: hfcpci_softirq: get func return to suppress compiler warning
From: Antonio Alecrim Jr @ 2013-09-14 17:20 UTC (permalink / raw)
To: Karsten Keil, Masanari Iida, netdev, linux-kernel; +Cc: Antonio Alecrim Jr
Signed-off-by: Antonio Alecrim Jr <antonio.alecrim@gmail.com>
---
drivers/isdn/hardware/mISDN/hfcpci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
index 7f910c7..3c92780 100644
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -2295,8 +2295,8 @@ _hfcpci_softirq(struct device *dev, void *arg)
static void
hfcpci_softirq(void *arg)
{
- (void) driver_for_each_device(&hfc_driver.driver, NULL, arg,
- _hfcpci_softirq);
+ WARN_ON_ONCE(driver_for_each_device(&hfc_driver.driver, NULL, arg,
+ _hfcpci_softirq) != 0);
/* if next event would be in the past ... */
if ((s32)(hfc_jiffies + tics - jiffies) <= 0)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] bridge: use br_port_get_rtnl within rtnl lock
From: Eric Dumazet @ 2013-09-14 18:46 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, vyasevic, Hong Zhiguo
In-Reply-To: <1379169748-767-2-git-send-email-zhiguohong@tencent.com>
On Sat, 2013-09-14 at 22:42 +0800, Hong Zhiguo wrote:
> From: Hong Zhiguo <zhiguohong@tencent.com>
>
> current br_port_get_rcu is problematic in bridging path
> (NULL deref). Change these calls in netlink path first.
>
> Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
> ---
> net/bridge/br_netlink.c | 4 ++--
> net/bridge/br_private.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
This is not net-next patch, but suitable for net tree, as the following
one fixes a potential panic.
^ permalink raw reply
* Re: [PATCH net-next 2/2] bridge: fix NULL pointer deref of br_port_get_rcu
From: Eric Dumazet @ 2013-09-14 18:47 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, vyasevic, Hong Zhiguo
In-Reply-To: <1379169748-767-3-git-send-email-zhiguohong@tencent.com>
On Sat, 2013-09-14 at 22:42 +0800, Hong Zhiguo wrote:
> From: Hong Zhiguo <zhiguohong@tencent.com>
>
> The NULL deref happens when br_handle_frame is called between these
> 2 lines of del_nbp:
> dev->priv_flags &= ~IFF_BRIDGE_PORT;
> /* --> br_handle_frame is called at this time */
> netdev_rx_handler_unregister(dev);
>
> In br_handle_frame the return of br_port_get_rcu(dev) is dereferenced
> without check but br_port_get_rcu(dev) returns NULL if:
> !(dev->priv_flags & IFF_BRIDGE_PORT)
>
> Eric Dumazet pointed out the testing of IFF_BRIDGE_PORT is not necessary
> here since we're in rcu_read_lock and we have synchronize_net() in
> netdev_rx_handler_unregister. So remove the testing of IFF_BRIDGE_PORT
> and by the previous patch, make sure br_port_get_rcu is called in
> bridging code.
>
> Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
> ---
> net/bridge/br_private.h | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
Again this is suitable for net tree.
Thanks !
^ permalink raw reply
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
From: Or Gerlitz @ 2013-09-14 19:10 UTC (permalink / raw)
To: Prarit Bhargava, Jack Morgenstein
Cc: netdev@vger.kernel.org, Doug Ledford, Amir Vadai, Or Gerlitz
In-Reply-To: <1379075438-20240-1-git-send-email-prarit@redhat.com>
On Fri, Sep 13, 2013 at 3:30 PM, Prarit Bhargava <prarit@redhat.com> wrote:
> Fix unitialized variable warnings.
Hi,
I'd like Jack, who is in charge on this code to take a look, added him.
Or.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&cq->mtt->ref_count);
> ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&srq->mtt->ref_count);
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: Amir Vadai <amirv@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx4/resource_tracker.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index dd68763..d703838 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -2563,7 +2563,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> {
> int err;
> int cqn = vhcr->in_modifier;
> - struct res_cq *cq;
> + struct res_cq *uninitialized_var(cq);
>
> err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> if (err)
> @@ -2746,7 +2746,7 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> {
> int err;
> int srqn = vhcr->in_modifier;
> - struct res_srq *srq;
> + struct res_srq *uninitialized_var(srq);
>
> err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
> if (err)
> --
> 1.7.9.3
>
> --
> 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
^ permalink raw reply
* Re: [PATCH] ptp: measure the time offset between PHC and system clock
From: Sergei Shtylyov @ 2013-09-14 19:21 UTC (permalink / raw)
To: Dong Zhu; +Cc: Richard Cochran, Rob Landley, netdev, linux-doc, linux-kernel
In-Reply-To: <20130914080306.GC23682@zhudong.nay.redhat.com>
Hello.
On 09/14/2013 12:03 PM, Dong Zhu wrote:
> This patch add a method into testptp.c to measure the time offset
> between phc and system clock through the ioctl PTP_SYS_OFFSET.
> Signed-off-by: Dong Zhu <bluezhudong@gmail.com>
> ---
> Documentation/ptp/testptp.c | 40 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
> diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c
> index f59ded0..72bb030 100644
> --- a/Documentation/ptp/testptp.c
> +++ b/Documentation/ptp/testptp.c
[...]
> @@ -376,6 +387,31 @@ int main(int argc, char *argv[])
> }
> }
>
> + if (offset) {
> + sysoff = calloc(1, sizeof(*sysoff));
> + if (!sysoff) {
> + perror("calloc");
> + return -1;
> + }
> + sysoff->n_samples = n_samples;
> +
> + if (ioctl(fd, PTP_SYS_OFFSET, sysoff))
> + perror("PTP_SYS_OFFSET");
> + else
> + puts("time offset between PHC and
> + system clock request okay");
Don't break the string constant that way, there'll be all spaces between
"and" and "system" included in it. Do it like this:
puts("time offset between PHC and "
"system clock request okay");
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 6/7] rtlwifi: Fix smatch warnings in usb.c
From: Sergei Shtylyov @ 2013-09-14 19:44 UTC (permalink / raw)
To: Larry Finger
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379094304-22041-7-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Hello.
On 09/13/2013 09:45 PM, Larry Finger wrote:
> Smatch displays the following:
> CHECK drivers/net/wireless/rtlwifi/usb.c
> drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn: assigning (-98) to unsigned variable 'stats.noise'
> drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() warn: assigning (-98) to unsigned variable 'stats.noise'
> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
> The negative number to an unsigned quantity is fixed by adding 256 to -98
> to get the equivalent negative number.
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> ---
> drivers/net/wireless/rtlwifi/usb.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
> diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
> index e56778c..9f3dcb8 100644
> --- a/drivers/net/wireless/rtlwifi/usb.c
> +++ b/drivers/net/wireless/rtlwifi/usb.c
[...]
> @@ -582,12 +582,15 @@ static void _rtl_rx_work(unsigned long param)
> static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
> unsigned int len)
> {
> +#if NET_IP_ALIGN != 0
> unsigned int padding = 0;
> +#endif
>
> /* make function no-op when possible */
> - if (NET_IP_ALIGN == 0 || len < sizeof(*hdr))
> + if (NET_IP_ALIGN == 0 || len < sizeof(struct ieee80211_hdr))
Why this collateral and undocumented change? What does it achieve?
> return 0;
WBR, Sergei
--
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
* Re: [PATCH 2/7] rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
From: Sergei Shtylyov @ 2013-09-14 19:59 UTC (permalink / raw)
To: Larry Finger
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379094304-22041-3-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Hello.
On 09/13/2013 09:44 PM, Larry Finger wrote:
> Smatch lists the following:
> CHECK drivers/net/wireless/rtlwifi/rtl8192de/hw.c
> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
> Dead code is removed.
It is instead commented out, including non-dead code it seems...
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> ---
> drivers/net/wireless/rtlwifi/rtl8192de/hw.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
> index 7dd8f6d..c9b0894 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
> @@ -1194,6 +1194,7 @@ void rtl92d_linked_set_reg(struct ieee80211_hw *hw)
> * mac80211 will send pkt when scan */
> void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
> {
> +/*
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> rtl92d_dm_init_edca_turbo(hw);
> return;
Shouldn't the comment start here (and *return* removed)? It's also
better to remove the dead code than just to comment it out.
> @@ -1213,6 +1214,7 @@ void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
> RT_ASSERT(false, "invalid aci: %d !\n", aci);
> break;
> }
> + */
> }
WBR, Sergei
--
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
* Re: [PATCH 6/7] rtlwifi: Fix smatch warnings in usb.c
From: Larry Finger @ 2013-09-14 20:26 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5234BCB6.6050508-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
On 09/14/2013 02:44 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 09/13/2013 09:45 PM, Larry Finger wrote:
>
>> Smatch displays the following:
>> CHECK drivers/net/wireless/rtlwifi/usb.c
>> drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn:
>> assigning (-98) to unsigned variable 'stats.noise'
>> drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() warn:
>> assigning (-98) to unsigned variable 'stats.noise'
>> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring
>> unreachable code.
>> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring
>> unreachable code.
>
>> The negative number to an unsigned quantity is fixed by adding 256 to -98
>> to get the equivalent negative number.
>
>> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
>> ---
>> drivers/net/wireless/rtlwifi/usb.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>> diff --git a/drivers/net/wireless/rtlwifi/usb.c
>> b/drivers/net/wireless/rtlwifi/usb.c
>> index e56778c..9f3dcb8 100644
>> --- a/drivers/net/wireless/rtlwifi/usb.c
>> +++ b/drivers/net/wireless/rtlwifi/usb.c
> [...]
>> @@ -582,12 +582,15 @@ static void _rtl_rx_work(unsigned long param)
>> static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
>> unsigned int len)
>> {
>> +#if NET_IP_ALIGN != 0
>> unsigned int padding = 0;
>> +#endif
>>
>> /* make function no-op when possible */
>> - if (NET_IP_ALIGN == 0 || len < sizeof(*hdr))
>> + if (NET_IP_ALIGN == 0 || len < sizeof(struct ieee80211_hdr))
>
> Why this collateral and undocumented change? What does it achieve?
>
>> return 0;
It does not change a thing. Further up the code is "struct ieee80211_hdr *hdr".
This change was one I tried during the fixes, and I forgot to remove it.
Larry
--
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
* Re: [PATCH 2/7] rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
From: Larry Finger @ 2013-09-14 20:30 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linville, linux-wireless, netdev
In-Reply-To: <5234C03A.2070701@cogentembedded.com>
On 09/14/2013 02:59 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 09/13/2013 09:44 PM, Larry Finger wrote:
>
>> Smatch lists the following:
>> CHECK drivers/net/wireless/rtlwifi/rtl8192de/hw.c
>> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info:
>> ignoring unreachable code.
>> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info:
>> ignoring unreachable code.
>
>> Dead code is removed.
>
> It is instead commented out, including non-dead code it seems...
>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>> drivers/net/wireless/rtlwifi/rtl8192de/hw.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
>> b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
>> index 7dd8f6d..c9b0894 100644
>> --- a/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
>> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
>> @@ -1194,6 +1194,7 @@ void rtl92d_linked_set_reg(struct ieee80211_hw *hw)
>> * mac80211 will send pkt when scan */
>> void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
>> {
>> +/*
>> struct rtl_priv *rtlpriv = rtl_priv(hw);
>> rtl92d_dm_init_edca_turbo(hw);
>> return;
>
> Shouldn't the comment start here (and *return* removed)? It's also
> better to remove the dead code than just to comment it out.
That would leave some unused variables.
>> @@ -1213,6 +1214,7 @@ void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
>> RT_ASSERT(false, "invalid aci: %d !\n", aci);
>> break;
>> }
>> + */
>> }
I'm not sure what that unreachable code might do, thus I saved it as a comment
for possible future use. I need to do further evaluation on this fragment, and
probably consult with the Realtek engineers.
Larry
^ permalink raw reply
* Re: [RFC PATCH 2/4] net: Add lower dev list helpers
From: John Fastabend @ 2013-09-14 20:43 UTC (permalink / raw)
To: Veaceslav Falico
Cc: stephen, bhutchings, ogerlitz, john.ronciak, netdev,
shannon.nelson
In-Reply-To: <20130914122756.GA18327@redhat.com>
On 09/14/2013 05:27 AM, Veaceslav Falico wrote:
> On Wed, Sep 11, 2013 at 11:46:49AM -0700, John Fastabend wrote:
>> This patch adds helpers to traverse the lower dev lists, these
>> helpers match the upper dev list implementation.
>>
>> VSI implementers may use these to track a list of connected netdevs.
>> This is easier then having drivers do their own accounting.
>
> Just as a note (as I have quite no idea how ixgbe works) - you are aware
> that the upper/lower lists currently include *all* upper/lower devices, and
> not only the first-connected ones?
Yep, the netif_is_vsi_port() should hopefully catch all the cases.
>
> I've seen that you usually verify it, however not always, just a heads-up -
> sorry if misread.
I'll audit it again, maybe I missed a case.
>
> I'm also currently trying to get the new patchset included - which would
> split the first-tier connected devices from all the 'other' (as in - lower
> of a lower and upper of an upper) devices, so that way, I think, it would
> be easier/faster for you to use it. It also has a ->private field, easily
> accessible, which you could have used instead of/in conjunction with/for
> struct ixgbe_vsi_adapter.
Yeah I saw the patch set I'll use the improved lists once I see the
series gets applied. Although in my case the list walking only occurs in
the configuration path so its not critical.
Also the net device structure already has private space to hang the
device dependent structure off of, so I'm not sure having another
private field helps here.
Thanks,
John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [RFC PATCH 2/4] net: Add lower dev list helpers
From: Veaceslav Falico @ 2013-09-14 21:14 UTC (permalink / raw)
To: John Fastabend
Cc: stephen, bhutchings, ogerlitz, john.ronciak, netdev,
shannon.nelson
In-Reply-To: <5234CA5D.5020603@gmail.com>
On Sat, Sep 14, 2013 at 01:43:09PM -0700, John Fastabend wrote:
>On 09/14/2013 05:27 AM, Veaceslav Falico wrote:
>>On Wed, Sep 11, 2013 at 11:46:49AM -0700, John Fastabend wrote:
>>>This patch adds helpers to traverse the lower dev lists, these
>>>helpers match the upper dev list implementation.
>>>
>>>VSI implementers may use these to track a list of connected netdevs.
>>>This is easier then having drivers do their own accounting.
>>
>>Just as a note (as I have quite no idea how ixgbe works) - you are aware
>>that the upper/lower lists currently include *all* upper/lower devices, and
>>not only the first-connected ones?
>
>Yep, the netif_is_vsi_port() should hopefully catch all the cases.
>
>>
>>I've seen that you usually verify it, however not always, just a heads-up -
>>sorry if misread.
>
>I'll audit it again, maybe I missed a case.
It was about lower devices, and I think I've just really misread it -
indeed they can be only 'your' devices.
>
>>
>>I'm also currently trying to get the new patchset included - which would
>>split the first-tier connected devices from all the 'other' (as in - lower
>>of a lower and upper of an upper) devices, so that way, I think, it would
>>be easier/faster for you to use it. It also has a ->private field, easily
>>accessible, which you could have used instead of/in conjunction with/for
>>struct ixgbe_vsi_adapter.
>
>Yeah I saw the patch set I'll use the improved lists once I see the
>series gets applied. Although in my case the list walking only occurs in
>the configuration path so its not critical.
>
>Also the net device structure already has private space to hang the
>device dependent structure off of, so I'm not sure having another
>private field helps here.
Yep, it should/can be used when linking devices which might already be
using their private space for their own structures, which is not your case,
I suppose.
>
>Thanks,
>John
>
>--
>John Fastabend Intel Corporation
^ 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