Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd
On 09/04/2015 10:41 PM, Kees Cook wrote:
> On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
> wrote:
>> This is the final bit needed to support seccomp filters created via the bpf
>> syscall.
Hmm. Thanks Kees, for CCinf linux-api@. That really should have been done at
the outset.
Tycho, where's the man-pages patch describing this new kernel-userspace
API feature? :-)
>> One concern with this patch is exactly what the interface should look like
>> for users, since seccomp()'s second argument is a pointer, we could ask
>> people to pass a pointer to the fd, but implies we might write to it which
>> seems impolite. Right now we cast the pointer (and force the user to cast
>> it), which generates ugly warnings. I'm not sure what the right answer is
>> here.
>>
>> Signed-off-by: Tycho Andersen
>> CC: Kees Cook
>> CC: Will Drewry
>> CC: Oleg Nesterov
>> CC: Andy Lutomirski
>> CC: Pavel Emelyanov
>> CC: Serge E. Hallyn
>> CC: Alexei Starovoitov
>> CC: Daniel Borkmann
>> ---
>> include/linux/seccomp.h | 3 +-
>> include/uapi/linux/seccomp.h | 1 +
>> kernel/seccomp.c | 70
>>
>> 3 files changed, 61 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
>> index d1a86ed..a725dd5 100644
>> --- a/include/linux/seccomp.h
>> +++ b/include/linux/seccomp.h
>> @@ -3,7 +3,8 @@
>>
>> #include
>>
>> -#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC)
>> +#define SECCOMP_FILTER_FLAG_MASK (\
>> + SECCOMP_FILTER_FLAG_TSYNC | SECCOMP_FILTER_FLAG_EBPF)
>>
>> #ifdef CONFIG_SECCOMP
>>
>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>> index 0f238a4..c29a423 100644
>> --- a/include/uapi/linux/seccomp.h
>> +++ b/include/uapi/linux/seccomp.h
>> @@ -16,6 +16,7 @@
>>
>> /* Valid flags for SECCOMP_SET_MODE_FILTER */
>> #define SECCOMP_FILTER_FLAG_TSYNC 1
>> +#define SECCOMP_FILTER_FLAG_EBPF (1 << 1)
>>
>> /*
>> * All BPF programs must return a 32-bit value.
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index a2c5b32..9c6bea6 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -355,17 +355,6 @@ static struct seccomp_filter
>> *seccomp_prepare_filter(struct sock_fprog *fprog)
>>
>> BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
>>
>> - /*
>> -* Installing a seccomp filter requires that the task has
>> -* CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
>> -* This avoids scenarios where unprivileged tasks can affect the
>> -* behavior of privileged children.
>> -*/
>> - if (!task_no_new_privs(current) &&
>> - security_capable_noaudit(current_cred(), current_user_ns(),
>> -CAP_SYS_ADMIN) != 0)
>> - return ERR_PTR(-EACCES);
>> -
>> /* Allocate a new seccomp_filter */
>> sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
>> if (!sfilter)
>> @@ -509,6 +498,48 @@ static void seccomp_send_sigsys(int syscall, int reason)
>> info.si_syscall = syscall;
>> force_sig_info(SIGSYS, &info, current);
>> }
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static struct seccomp_filter *seccomp_prepare_ebpf(const char __user
>> *filter)
>> +{
>> + /* XXX: this cast generates a warning. should we make people pass in
>> +* &fd, or is there some nicer way of doing this?
>> +*/
>> + u32 fd = (u32) filter;
>
> I think this is probably the right way to do it, modulo getting the
> warning fixed. Let me invoke the great linux-api subscribers to get
> some more opinions.
Sigh. It's sad, but the using a cast does seem the simplest option.
But, how about another idea...
> tl;dr: adding SECCOMP_FILTER_FLAG_EBPF to the flags changes the
> pointer argument into an fd argument. Is this sane, should it be a
> pointer to an fd, or should it not be a flag at all, creating a new
> seccomp command instead (SECCOMP_MODE_FILTER_EBPF)?
What about
seccomp(SECCOMP_MODE_FILTER_EBPF, flags, structp)
Where structp is a pointer to something like
struct seccomp_ebpf {
int size; /* Size of this whole struct */
int fd;
}
'size' allows for future expansion of the struct (in case we want to
expand it later), and placing 'fd' inside a struct avoids unpleasant
implication that would be made by passing a pointer to an fd as the
third argument.
Cheers,
Michael
> -Kees
>
>> + struct seccomp_filter *ret;
>> + struct bpf_prog *prog;
>> +
>> + prog = bpf_prog_get(fd);
>> + if (IS_ERR(prog))
>> + return (struct seccomp_filter *) prog;
>> +
>> + if (prog->type != BPF_PROG_TYPE_SECCOMP) {
>> + bpf_prog_put(prog);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + ret = kzalloc(sizeof(*ret), GFP_KERNEL | __GFP_NOWARN);
>> + if (!ret) {
>> + bpf
[PATCH] iproute: print more verbose error on route cache flush
Before:
kda@vfirst ~/devel/iproute2 $ ./ip/ip route flush cache
Cannot open "/proc/sys/net/ipv4/route/flush"
After:
kda@vfirst ~/devel/iproute2/ip $ ./ip route flush cache
Cannot open "/proc/sys/net/ipv4/route/flush": Permission denied
Signed-off-by: Denis Kirjanov
---
ip/iproute.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index 8f49e62..abe4180 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1213,7 +1213,8 @@ static int iproute_flush_cache(void)
char *buffer = "-1";
if (flush_fd < 0) {
- fprintf (stderr, "Cannot open \"%s\"\n", ROUTE_FLUSH_PATH);
+ fprintf (stderr, "Cannot open \"%s\": %s\n",
+ ROUTE_FLUSH_PATH, strerror(errno));
return -1;
}
--
2.4.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 1/4] Add correlated clocksource deriving system time from an auxiliary clocksource
On Fri, 4 Sep 2015, Hall, Christopher S wrote: > > Right, but we still do not know how that is going to be used. And > > that's the key question. As long as that is not answered all can do is > > wild guessing. > > It's not wild guessing. We do have it working on other OSs and have > a pretty good idea of how it will work. The DSP firmware will be > largely identical for Linux. I think now, we have a chicken and egg > problem. You have a totally different problem. You are just refusing to explain how all that stuff is supposed to work and what kind of functionality you need exactly. Is it that hard to describe the technical requirements and the presumably assbackwards restrictions of the firmware? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] Add correlated clocksource deriving system time from an auxiliary clocksource
* Thomas Gleixner wrote: > On Fri, 4 Sep 2015, Hall, Christopher S wrote: > > > Right, but we still do not know how that is going to be used. And > > > that's the key question. As long as that is not answered all can do is > > > wild guessing. > > > > It's not wild guessing. We do have it working on other OSs and have a > > pretty > > good idea of how it will work. The DSP firmware will be largely identical > > for > > Linux. I think now, we have a chicken and egg problem. > > You have a totally different problem. You are just refusing to explain how > all > that stuff is supposed to work and what kind of functionality you need > exactly. Yeah, so I'm just going to NAK this until things are improved: NAKed-by: Ingo Molnar it's not like we are overly bored in timekeeping and need the extra complexity as much as possible. Proper, comprehensive, proactive technical description is needed, with proper changelogs, not just half-baked notes. If all that is fixed I'll lift my NAK. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
On Wed, 2015-08-05 at 15:01 +0400, Ivan Mikhaylov wrote: > * do the redefinition of emac_regs struct from driver structure perspective > and passing size from actual struct size, not from memory area variable > which set in dts file. > > * passing variable from dts option may cause a problem with output below > MII's section which we're fixing with this and 5369c71 commit in kernel. [...] But you still aren't handling the case where only one of the driver and ethtool is upgraded, even by reporting an error. I'm never going to apply patches to ethtool that obviously break binary compatibility, so you are wasting your time sending me new versions that do that. At this point you should probably just bump the dump version numbers in both places. Ben. -- Ben Hutchings friends: People who know you well, but like you anyway. signature.asc Description: This is a digitally signed message part
Re: parsing the output of ethtool programmatically
On Thu, 2015-06-25 at 03:28 +0200, Robert Urban wrote: > Hello, > > there doesn't seem to be any flag to request ethtool to present its output in > a > script-friendly form. Things like: > > Link partner advertised link modes: 10baseT/Half 10baseT/Full > > 100baseT/Half 100baseT/Full > > 1000baseT/Full > are kind of pain to parse. > > Something to consider? Or write a library for using SIOCETHTOOL. The interface is fairly well documented now and there are other users besides the ethtool utility. Ben. -- Ben Hutchings friends: People who know you well, but like you anyway. signature.asc Description: This is a digitally signed message part
Re: [PATCH ethtool V3 1/2] ethtool: Add copybreak support
On Thu, 2015-06-11 at 15:15 +0300, Hadar Hen Zion wrote: > From: Govindarajulu Varadarajan <_gov...@gmx.com> > > Add support for setting/getting driver's tx/rx_copybreak value. > > Copybreak is handled through a new ethtool tunable interface. > > The kernel support was added in 3.18, commit f0db9b07341 "ethtool: > Add generic options for tunables" [...] So why have you sent a patch that only handles copy-break? The kernel tells us the type of each tunable and ethtool should use that to decide how to print and parse values. Ben. -- Ben Hutchings friends: People who know you well, but like you anyway. signature.asc Description: This is a digitally signed message part
Re: [PATCH ethtool V3 1/2] ethtool: Add copybreak support
On Thu, 2015-06-11 at 15:15 +0300, Hadar Hen Zion wrote: > From: Govindarajulu Varadarajan <_gov...@gmx.com> > > Add support for setting/getting driver's tx/rx_copybreak value. > > Copybreak is handled through a new ethtool tunable interface. > > The kernel support was added in 3.18, commit f0db9b07341 "ethtool: > Add generic options for tunables" [...] So why have you sent a patch that only handles copy-break? The kernel tells us the type of each tunable and ethtool should use that to decide how to print and parse values. Ben. -- Ben Hutchings friends: People who know you well, but like you anyway. signature.asc Description: This is a digitally signed message part
Re: [RFC PATCH 0/3] Network stack, first user of SLAB/kmem_cache bulk free API.
On Fri, 4 Sep 2015 18:45:13 -0500 (CDT) Christoph Lameter wrote: > On Fri, 4 Sep 2015, Alexander Duyck wrote: > > Right, but one of the reasons for Jesper to implement the bulk alloc/free is > > to avoid the cmpxchg that is being used to get stuff into or off of the per > > cpu lists. > > There is no full cmpxchg used for the per cpu lists. Its a cmpxchg without > lock semantics which is very cheap. The double_cmpxchg without lock prefix still cost 9 cycles, which is very fast but still a cost (add approx 19 cycles for a lock prefix). It is slower than local_irq_disable + local_irq_enable that only cost 7 cycles, which the bulking call uses. (That is the reason bulk calls with 1 object can almost compete with fastpath). > > In the case of network drivers they are running in softirq context almost > > exclusively. As such it is useful to have a set of buffers that can be > > acquired or freed from this context without the need to use any > > synchronization primitives. Then once the softirq context ends then we can > > free up some or all of the resources back to the slab allocator. > > That is the case in the slab allocators. There is a potential for taking advantage of this softirq context, which is basically what my qmempool implementation did. But we have now optimized the slub allocator to an extend that (in case of slab-tuning or slab_nomerge) is faster than my qmempool implementation. Thus, I would like a smaller/slimmer layer than qmempool. We do need some per CPU cache for allocations, like Alex suggests, but I'm not sure we need that for the free side. For now I'm returning objects/skbs directly to slub, and is hoping enough objects can be merged in a detached freelist, which allow me to return several objects with a single locked double_cmpxchg. -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 0/3] r8169: fix sleepable allocation during netdevice stats retrieval.
From: Francois Romieu
This series applies against davem's net as of
724a7636ad026a3a68f3fc626ccd04111f65cfd9 ("Merge branch 'sctp-fixes').
It's untested though reviewable. I should be able to try it this evening.
Francois Romieu (3):
r8169: decouple the counters data and the device private area.
r8169: move rtl_reset_counters_cond before the hardware counters helpers.
r8169: increase the lifespan of the hardware counters dump area.
drivers/net/ethernet/realtek/r8169.c | 200 +--
1 file changed, 96 insertions(+), 104 deletions(-)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 3/3] r8169: increase the lifespan of the hardware counters dump area.
From: Francois Romieu
net/core/net-sysfs.c::netstat_show retrieves stats with spinlock held.
This change avoids sleepable allocation and performs some housekeeping:
- receive ring, transmit ring and counters dump area allocation failures
are all considered fatal during open()
- netif_warn is now redundant with rtl_reset_counters_cond built-in
failure message.
- rtl_reset_counters_cond induced failures in open() are also considered
fatal: it takes acceptable work to unwind comfortably.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=104031
Fixes: 6e85d5ad36a2 ("r8169: Add values missing in @get_stats64 from HW
counters")
Signed-off-by: Francois Romieu
Cc: Corinna Vinschen
Cc: pomidorabelis...@gmail.com
---
drivers/net/ethernet/realtek/r8169.c | 132 +++
1 file changed, 55 insertions(+), 77 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c
b/drivers/net/ethernet/realtek/r8169.c
index c0a5edb..ae07567 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -833,8 +833,11 @@ struct rtl8169_private {
unsigned features;
struct mii_if_info mii;
+
struct rtl8169_counters *counters;
struct rtl8169_tc_offsets tc_offset;
+ dma_addr_t counters_map;
+
u32 saved_wolopts;
u32 opts1_mask;
@@ -2197,64 +2200,33 @@ DECLARE_RTL_COND(rtl_reset_counters_cond)
return RTL_R32(CounterAddrLow) & CounterReset;
}
-static struct rtl8169_counters *rtl8169_map_counters(struct net_device *dev,
-dma_addr_t *paddr,
-u32 counter_cmd)
+static int rtl8169_cmd_counters(struct net_device *dev, u32 counter_cmd)
{
struct rtl8169_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
- struct device *d = &tp->pci_dev->dev;
- struct rtl8169_counters *counters;
+ dma_addr_t paddr = tp->counters_map;
u32 cmd;
- counters = dma_alloc_coherent(d, sizeof(*counters), paddr, GFP_KERNEL);
- if (counters) {
- RTL_W32(CounterAddrHigh, (u64)*paddr >> 32);
- cmd = (u64)*paddr & DMA_BIT_MASK(32);
- RTL_W32(CounterAddrLow, cmd);
- RTL_W32(CounterAddrLow, cmd | counter_cmd);
- }
- return counters;
-}
+ RTL_W32(CounterAddrHigh, (u64)paddr >> 32);
+ cmd = (u64)paddr & DMA_BIT_MASK(32);
+ RTL_W32(CounterAddrLow, cmd);
+ RTL_W32(CounterAddrLow, cmd | counter_cmd);
-static void rtl8169_unmap_counters (struct net_device *dev,
- dma_addr_t paddr,
- struct rtl8169_counters *counters)
-{
- struct rtl8169_private *tp = netdev_priv(dev);
- void __iomem *ioaddr = tp->mmio_addr;
- struct device *d = &tp->pci_dev->dev;
-
- RTL_W32(CounterAddrLow, 0);
- RTL_W32(CounterAddrHigh, 0);
-
- dma_free_coherent(d, sizeof(*counters), counters, paddr);
+ return rtl_udelay_loop_wait_low(tp, &rtl_reset_counters_cond, 10, 1000);
}
-static bool rtl8169_reset_counters(struct net_device *dev)
+static int rtl8169_reset_counters(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
- struct rtl8169_counters *counters;
- dma_addr_t paddr;
- bool ret = true;
/*
* Versions prior to RTL_GIGA_MAC_VER_19 don't support resetting the
* tally counters.
*/
if (tp->mac_version < RTL_GIGA_MAC_VER_19)
- return true;
-
- counters = rtl8169_map_counters(dev, &paddr, CounterReset);
- if (!counters)
- return false;
-
- if (!rtl_udelay_loop_wait_low(tp, &rtl_reset_counters_cond, 10, 1000))
- ret = false;
-
- rtl8169_unmap_counters(dev, paddr, counters);
+ return -EINVAL;
- return ret;
+ return rtl8169_cmd_counters(dev, CounterReset);
}
DECLARE_RTL_COND(rtl_counters_cond)
@@ -2264,44 +2236,30 @@ DECLARE_RTL_COND(rtl_counters_cond)
return RTL_R32(CounterAddrLow) & CounterDump;
}
-static bool rtl8169_update_counters(struct net_device *dev)
+static int rtl8169_update_counters(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
- struct rtl8169_counters *counters;
- dma_addr_t paddr;
- bool ret = true;
/*
* Some chips are unable to dump tally counters when the receiver
* is disabled.
*/
if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0)
- return true;
-
- counters = rtl8169_map_counters(dev, &paddr, CounterDump);
- if (!counters)
- return false;
-
- if (rtl_udelay_loop_wait_low(tp, &rtl_counters_cond, 10, 1000))
- memcpy(&tp->counters, counters, sizeof(*counters));
- else
-
[PATCH net 2/3] r8169: move rtl_reset_counters_cond before the hardware counters helpers.
From: Francois Romieu
Signed-off-by: Francois Romieu
Cc: Corinna Vinschen
Cc: pomidorabelis...@gmail.com
---
drivers/net/ethernet/realtek/r8169.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c
b/drivers/net/ethernet/realtek/r8169.c
index 73a60a7..c0a5edb 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -2190,6 +2190,13 @@ static int rtl8169_get_sset_count(struct net_device
*dev, int sset)
}
}
+DECLARE_RTL_COND(rtl_reset_counters_cond)
+{
+ void __iomem *ioaddr = tp->mmio_addr;
+
+ return RTL_R32(CounterAddrLow) & CounterReset;
+}
+
static struct rtl8169_counters *rtl8169_map_counters(struct net_device *dev,
dma_addr_t *paddr,
u32 counter_cmd)
@@ -2224,13 +2231,6 @@ static void rtl8169_unmap_counters (struct net_device
*dev,
dma_free_coherent(d, sizeof(*counters), counters, paddr);
}
-DECLARE_RTL_COND(rtl_reset_counters_cond)
-{
- void __iomem *ioaddr = tp->mmio_addr;
-
- return RTL_R32(CounterAddrLow) & CounterReset;
-}
-
static bool rtl8169_reset_counters(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 1/3] r8169: decouple the counters data and the device private area.
From: Francois Romieu
Signed-off-by: Francois Romieu
Cc: Corinna Vinschen
Cc: pomidorabelis...@gmail.com
---
drivers/net/ethernet/realtek/r8169.c | 70 +---
1 file changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c
b/drivers/net/ethernet/realtek/r8169.c
index 24dcbe6..73a60a7 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -833,7 +833,7 @@ struct rtl8169_private {
unsigned features;
struct mii_if_info mii;
- struct rtl8169_counters counters;
+ struct rtl8169_counters *counters;
struct rtl8169_tc_offsets tc_offset;
u32 saved_wolopts;
u32 opts1_mask;
@@ -2296,6 +2296,8 @@ static bool rtl8169_update_counters(struct net_device
*dev)
static bool rtl8169_init_counter_offsets(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_counters *counters = tp->counters;
+ struct rtl8169_tc_offsets *offset = &tp->tc_offset;
bool ret = false;
/*
@@ -2313,7 +2315,7 @@ static bool rtl8169_init_counter_offsets(struct
net_device *dev)
* set at open time by rtl_hw_start.
*/
- if (tp->tc_offset.inited)
+ if (offset->inited)
return true;
/* If both, reset and update fail, propagate to caller. */
@@ -2323,10 +2325,10 @@ static bool rtl8169_init_counter_offsets(struct
net_device *dev)
if (rtl8169_update_counters(dev))
ret = true;
- tp->tc_offset.tx_errors = tp->counters.tx_errors;
- tp->tc_offset.tx_multi_collision = tp->counters.tx_multi_collision;
- tp->tc_offset.tx_aborted = tp->counters.tx_aborted;
- tp->tc_offset.inited = true;
+ offset->tx_errors = counters->tx_errors;
+ offset->tx_multi_collision = counters->tx_multi_collision;
+ offset->tx_aborted = counters->tx_aborted;
+ offset->inited = true;
return ret;
}
@@ -2335,24 +2337,25 @@ static void rtl8169_get_ethtool_stats(struct net_device
*dev,
struct ethtool_stats *stats, u64 *data)
{
struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_counters *c = tp->counters;
ASSERT_RTNL();
rtl8169_update_counters(dev);
- data[0] = le64_to_cpu(tp->counters.tx_packets);
- data[1] = le64_to_cpu(tp->counters.rx_packets);
- data[2] = le64_to_cpu(tp->counters.tx_errors);
- data[3] = le32_to_cpu(tp->counters.rx_errors);
- data[4] = le16_to_cpu(tp->counters.rx_missed);
- data[5] = le16_to_cpu(tp->counters.align_errors);
- data[6] = le32_to_cpu(tp->counters.tx_one_collision);
- data[7] = le32_to_cpu(tp->counters.tx_multi_collision);
- data[8] = le64_to_cpu(tp->counters.rx_unicast);
- data[9] = le64_to_cpu(tp->counters.rx_broadcast);
- data[10] = le32_to_cpu(tp->counters.rx_multicast);
- data[11] = le16_to_cpu(tp->counters.tx_aborted);
- data[12] = le16_to_cpu(tp->counters.tx_underun);
+ data[ 0] = le64_to_cpu(c->tx_packets);
+ data[ 1] = le64_to_cpu(c->rx_packets);
+ data[ 2] = le64_to_cpu(c->tx_errors);
+ data[ 3] = le32_to_cpu(c->rx_errors);
+ data[ 4] = le16_to_cpu(c->rx_missed);
+ data[ 5] = le16_to_cpu(c->align_errors);
+ data[ 6] = le32_to_cpu(c->tx_one_collision);
+ data[ 7] = le32_to_cpu(c->tx_multi_collision);
+ data[ 8] = le64_to_cpu(c->rx_unicast);
+ data[ 9] = le64_to_cpu(c->rx_broadcast);
+ data[10] = le32_to_cpu(c->rx_multicast);
+ data[11] = le16_to_cpu(c->tx_aborted);
+ data[12] = le16_to_cpu(c->tx_underun);
}
static void rtl8169_get_strings(struct net_device *dev, u32 stringset, u8
*data)
@@ -7671,6 +7674,8 @@ static int rtl8169_close(struct net_device *dev)
free_irq(pdev->irq, dev);
+ kfree(tp->counters);
+
dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
tp->RxPhyAddr);
dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
@@ -7715,9 +7720,13 @@ static int rtl_open(struct net_device *dev)
if (!tp->RxDescArray)
goto err_free_tx_0;
+ tp->counters = kmalloc(sizeof(*tp->counters), GFP_KERNEL);
+ if (!tp->counters)
+ goto err_free_rx_1;
+
retval = rtl8169_init_ring(dev);
if (retval < 0)
- goto err_free_rx_1;
+ goto err_free_counters_2;
INIT_WORK(&tp->wk.work, rtl_task);
@@ -7729,7 +7738,7 @@ static int rtl_open(struct net_device *dev)
(tp->features & RTL_FEATURE_MSI) ? 0 : IRQF_SHARED,
dev->name, dev);
if (retval < 0)
- goto err_release_fw_2;
+ goto err_release_fw_3;
rtl_lock_work(tp);
@@ -7759,9 +7768,12 @@ static int rtl_open(struct ne
FROM MR. MARTIN IBANEZ
Please view the attached file MR. MARTIN IBANEZ.pdf Description: Adobe PDF document
Re: [GIT] Networking
Hi all,
> On Wed, Sep 2, 2015 at 10:35 PM, David Miller wrote:
>>
>> Another merge window, another set of networking changes. I've heard
>> rumblings that the lightweight tunnels infrastructure has been voted
>> networking change of the year.
>
> .. and others say that the most notable feature is the idiotic bugs
> that it introduces, and the compiler even complains about.
>
> Christ, people. Learn C, instead of just stringing random characters
> together until it compiles (with warnings).
>
> This:
>
> static bool rate_control_cap_mask(struct ieee80211_sub_if_data *sdata,
>struct ieee80211_supported_band *sband,
>struct ieee80211_sta *sta, u32 *mask,
>u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
>
> is horribly broken to begin with, because array arguments in C don't
> actually exist. Sadly, compilers accept it for various bad historical
> reasons, and silently turn it into just a pointer argument. There are
> arguments for them, but they are from weak minds.
>
> But happily gcc has a really really valid warning (kudos - I often end
> up ragging on the bad warnings gcc has, but this one is a keeper),
> because a few lines down the mistake then turns into pure and utter
> garbage.
>
> It's garbage that was basically encouraged by the first mistake
> (thinking that C allows array arguments), namely:
>
> for (i = 0; i < sizeof(mcs_mask); i++)
>
I moved rate_control_apply_mask() logic in rate_control_cap_mask() in
order to be applied in multiple code points (i.e.
rate_control_apply_mask_ratetbl()). Since I was using gcc version
4.9.2, the warning did not show up. Sorry for that bug.
> the "sizeof(mcs_mask)" is _shit_. Since array arguments don't actually
> exist in C, it is the size of the pointer, not the array. The first
> mistake makes the bug look like reasonable code. Although I'd argue
> that the code would actually be bad regardless, since "sizeof" is the
> size in bytes, and the code actually wants the number of entries (and
> we do have ARRAY_SIZE() for that).
>
> Sure, in this case the entries are just one byte each, so it would
> have *worked* had it not been for the array argument issue, but it's
> misleading and the code is just fundamentally buggy and nonsensical in
> two entirely different ways that fed on each other.
>
> That line should read
>
> for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
>
> and the argument should just have been declared as the pointer it actually is.
>
> A later patch then added onto the pile of manure by adding *another*
> broken array argument, but at least that one then used the proper loop
> for traversal of that array.
>
> The fact that I notice this bug from a very basic "let's just compile
> each pull request and make sure it isn't complete crap" is sad.
>
> Now, it *looks* like the code was just moved, and the "sizeof()" was
> initially correct (because it was a size of an actual array). Well, it
> was "correct" in the sense that it generated the right code, even if
> the whole confusion between "number of entries" and "size in bytes"
> was still there. Then it got moved and turned from "confused but
> happens to generate correct code" into "buggy pile of bovine manure".
> See commit 90c66bd2232a ("mac80211: remove ieee80211_tx_rate
> dependency in rate mask code").
>
> So I can see how this bug happened, and I am only slightly upset with
> Lorenzo who is the author of that commit.
>
> What I can't see is why the code has existed in at least two
> maintainer trees (Johannes' and David's) for a couple of weeks, and
> nobody cared about the new compiler warnings? And it was sent to me
> despite that new warning?
>
> I realy want people to take a really hard look at functions that use
> arrays as arguments. It really is very misleading, even if it can look
> "prettier", and some people will argue that it's "documentation" about
> how the pointer is a particular size. But it's neither. It's basically
> just lying about what is going on, and the only thing it documents is
> "I don't know how to C". Misleading documentation isn't documentation,
> it's a mistake.
>
> I see it in that file for at least the functions rate_idx_match_mask()
> and rate_control_cap_mask(). I tried - and failed - to come up with a
> reasonable grep pattern to try to see how common it is, and I'm too
> lazy to add some sparse check for it.
>
> Please people. When I see these kinds of obviously bogus code
> problems, that just makes me very upset. Because it makes me worry
> about all the non-obvious stuff that I miss. Sadly, this time I had
> pushed out the merge early (because I wanted to test the wireless
> changes on my laptop), so now the bug is out there.
>
> I'm not sure what the practical *impact* of the bug is. Yes, it only
> traverses four or eight rate entries (depending on 32-bit or
> 64-bitness of the kernel) out of the ten that it sho
[PATCH] net: phy: Handle postive return codes in phy_connect
The function phy_connect_direct can possibly return a positive return code. Using ERR_PTR with a positive value can lead to deferencing of an invalid pointer. Signed-off-by: Michael Welling --- drivers/net/phy/phy_device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index c0f2111..a7e14a6 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -477,6 +477,9 @@ struct phy_device *phy_connect(struct net_device *dev, const char *bus_id, phydev = to_phy_device(d); rc = phy_connect_direct(dev, phydev, handler, interface); + if (rc > 0) + return ERR_PTR(-ENODEV); + if (rc) return ERR_PTR(rc); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ethtool: Dump eeprom info for soldered on modules
On Sun, 2015-08-16 at 04:05 +0200, Andrew Lunn wrote: > Modules which are soldered onto the motherboard may also use the > sff8079 > EEPROM format. Dump these in the same way as SFP modules. > > Signed-off-by: Andrew Lunn [...] Applied, thanks. Ben. -- Ben Hutchings friends: People who know you well, but like you anyway. signature.asc Description: This is a digitally signed message part
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: > The function phy_connect_direct can possibly return a positive > return code. Using ERR_PTR with a positive value can lead to > deferencing of an invalid pointer. Is this the correct fix? Would it not be better to find where the positive return code is from and fix that? Andrew > Signed-off-by: Michael Welling > --- > drivers/net/phy/phy_device.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index c0f2111..a7e14a6 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -477,6 +477,9 @@ struct phy_device *phy_connect(struct net_device *dev, > const char *bus_id, > phydev = to_phy_device(d); > > rc = phy_connect_direct(dev, phydev, handler, interface); > + if (rc > 0) > + return ERR_PTR(-ENODEV); > + > if (rc) > return ERR_PTR(rc); > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
We offer both personal and business loan to any interested individual or company who will be able to pay back the loan when due for repayment. should Fill and return the application details below to my e-mail at ( montageglobalpvt...@gmail.com ). APPLICATION DETAILS 1) FULL NAMES: 2) CONTACT ADDRESS: 3) COUNTRY: 4) PHONE NUMBER: 5) SEX: 6) AGE: 7) AMOUNT NEEDED AS LOAN: 8) LOAN DURATION: 9) OCCUPATION: 10)MONTHLY INCOME: 11)PURPOSE OF LOAN: Warm regard. William Allen. Phone: +91-7838915074 (Chairman, Montage Global Pvt Ltd.) © 2015 Montage Global Pvt Ltd. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
On Sat, Sep 05, 2015 at 09:18:40PM +0200, Andrew Lunn wrote: > On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: > > The function phy_connect_direct can possibly return a positive > > return code. Using ERR_PTR with a positive value can lead to > > deferencing of an invalid pointer. > > Is this the correct fix? Would it not be better to find where the > positive return code is from and fix that? I guess I can trace it back to find out where the positive return code is originating. Is phy_connect_direct always supposed to return valid -errno? > >Andrew > > > Signed-off-by: Michael Welling > > --- > > drivers/net/phy/phy_device.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index c0f2111..a7e14a6 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -477,6 +477,9 @@ struct phy_device *phy_connect(struct net_device *dev, > > const char *bus_id, > > phydev = to_phy_device(d); > > > > rc = phy_connect_direct(dev, phydev, handler, interface); > > + if (rc > 0) > > + return ERR_PTR(-ENODEV); > > + > > if (rc) > > return ERR_PTR(rc); > > > > -- > > 2.1.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe netdev" in > > the body of a message to majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
On Sat, Sep 05, 2015 at 02:44:01PM -0500, Michael Welling wrote: > On Sat, Sep 05, 2015 at 09:18:40PM +0200, Andrew Lunn wrote: > > On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: > > > The function phy_connect_direct can possibly return a positive > > > return code. Using ERR_PTR with a positive value can lead to > > > deferencing of an invalid pointer. > > > > Is this the correct fix? Would it not be better to find where the > > positive return code is from and fix that? > > I guess I can trace it back to find out where the positive return code > is originating. > > Is phy_connect_direct always supposed to return valid -errno? I would look at this from a different angle. A positive ERRNO is probably a bug of some sort. So rather than papering over the cracks, go find what the real issue is. It might not be an ERRNO. E.g. https://lkml.org/lkml/2015/9/3/534 fixed a bug where a positive value is returned which is not an indication of an error. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
_DSD standardization note (WAS: Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding.)
Following up on this thread after finally seeing it...figured I would send something just for the archive mainly (we discussed this in person recently at a few different events and I think are aligned already). On 08/07/2015 08:28 PM, Rafael J. Wysocki wrote: > Hi David, > > On Sat, Aug 8, 2015 at 2:11 AM, David Daney wrote: >> On 08/07/2015 05:05 PM, Rafael J. Wysocki wrote: > > [cut] > >>> >>> It is actually useful to people as far as I can say. >>> >>> Also, if somebody is going to use properties with ACPI, why whould >>> they use a different set of properties with DT? Generally speaking, if there's a net new thing to handle, there is of course no particular problem with using DT as an inspiration, but we need to be conscious of the fact that Linux isn't the only Operating System that may need to support these bindings, so the correct thing (as stated by many of you, and below, and in person also recently - so we are aligned) is to get this (the MAC address binding for _DSD in ACPI) standardized properly through UEFI where everyone who has a vest OS interest beyond Linux can also have their own involvement as well. As discussed, that doesn't make it part of ACPI6.0, just a binding. FWIW I made a decision on the Red Hat end that in our guidelines to partners for ARM RHEL(SA - Server for ARM) builds we would not generally endorse any use of _DSD, with the exception of the MAC address binding being discussed here. In that case, I realized I had not been fully prescriptive enough with the vendors building early hw in that I should have realized this would happen and have required that they do this the right way. MAC IP should be more sophisticated such that it can handle being reset even after the firmware has loaded its MAC address(es). Platform flash storage separate from UEFI variable storage (which is being abused to contain too much now that DXE drivers then load into the ACPI tables prior to exiting Boot Services, etc.) should contain the actual MAC address(es), as it is done on other arches, and it should not be necessary to communicate this via ACPI tables to begin with (that's a cost saving embedded concept that should not happen on server systems in the general case). I have already had several unannounced future designs adjusted in light of this _DSD usage. In the case of providing MAC address information (only) by _DSD, I connected the initial ARMv8 SoC silicon vendors who needed to use such a hack to ensure they were using the same properties, and will followup off list to ensure Cavium are looped into that. But, we do need to get the _DSD property for MAC address provision standardized through UEFI properly as an official binding rather than just a link on the website, and then we need to be extremely careful not to grow any further dependence upon _DSD elsewhere. Generally, if you're using that approach on a server system (other than for this MAC case), your firmware or design (or both) need to be modified to not use _DSD. Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net] net: dsa: bcm_sf2: Fix ageing conditions and operation
The comparison check between cur_hw_state and hw_state is currently
invalid because cur_hw_state is right shifted by G_MISTP_SHIFT, while
hw_state is not, so we end-up comparing bits 2:0 with bits 7:5, which is
going to cause an additional aging to occur. Fix this by not shifting
cur_hw_state while reading it, but instead, mask the value with the
appropriately shitfted bitmask.
The other problem with the fast-ageing process is that we did not set
the EN_AGE_DYNAMIC bit to request the ageing to occur for dynamically
learned MAC addresses. Finally, write back 0 to the FAST_AGE_CTRL
register to avoid leaving spurious bits sets from one operation to the
other.
Fixes: 12f460f23423 ("net: dsa: bcm_sf2: add HW bridging support")
Signed-off-by: Florian Fainelli
---
David, this dates back to 4.1, could you queue this for -stable?
Thanks!
drivers/net/dsa/bcm_sf2.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 289e20443d83..9d56515f4c4d 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -418,7 +418,7 @@ static int bcm_sf2_sw_fast_age_port(struct dsa_switch *ds,
int port)
core_writel(priv, port, CORE_FAST_AGE_PORT);
reg = core_readl(priv, CORE_FAST_AGE_CTRL);
- reg |= EN_AGE_PORT | FAST_AGE_STR_DONE;
+ reg |= EN_AGE_PORT | EN_AGE_DYNAMIC | FAST_AGE_STR_DONE;
core_writel(priv, reg, CORE_FAST_AGE_CTRL);
do {
@@ -432,6 +432,8 @@ static int bcm_sf2_sw_fast_age_port(struct dsa_switch *ds,
int port)
if (!timeout)
return -ETIMEDOUT;
+ core_writel(priv, 0, CORE_FAST_AGE_CTRL);
+
return 0;
}
@@ -507,7 +509,7 @@ static int bcm_sf2_sw_br_set_stp_state(struct dsa_switch
*ds, int port,
u32 reg;
reg = core_readl(priv, CORE_G_PCTL_PORT(port));
- cur_hw_state = reg >> G_MISTP_STATE_SHIFT;
+ cur_hw_state = reg & (G_MISTP_STATE_MASK << G_MISTP_STATE_SHIFT);
switch (state) {
case BR_STATE_DISABLED:
@@ -531,10 +533,12 @@ static int bcm_sf2_sw_br_set_stp_state(struct dsa_switch
*ds, int port,
}
/* Fast-age ARL entries if we are moving a port from Learning or
-* Forwarding state to Disabled, Blocking or Listening state
+* Forwarding (cur_hw_state) state to Disabled, Blocking or Listening
+* state (hw_state)
*/
if (cur_hw_state != hw_state) {
- if (cur_hw_state & 4 && !(hw_state & 4)) {
+ if (cur_hw_state >= G_MISTP_LEARN_STATE &&
+ hw_state <= G_MISTP_LISTEN_STATE) {
ret = bcm_sf2_sw_fast_age_port(ds, port);
if (ret) {
pr_err("%s: fast-ageing failed\n", __func__);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
Le 09/05/15 12:47, Andrew Lunn a écrit : > On Sat, Sep 05, 2015 at 02:44:01PM -0500, Michael Welling wrote: >> On Sat, Sep 05, 2015 at 09:18:40PM +0200, Andrew Lunn wrote: >>> On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: The function phy_connect_direct can possibly return a positive return code. Using ERR_PTR with a positive value can lead to deferencing of an invalid pointer. >>> >>> Is this the correct fix? Would it not be better to find where the >>> positive return code is from and fix that? >> >> I guess I can trace it back to find out where the positive return code >> is originating. >> >> Is phy_connect_direct always supposed to return valid -errno? > > I would look at this from a different angle. A positive ERRNO is > probably a bug of some sort. So rather than papering over the cracks, > go find what the real issue is. Agreed, you could place a WARN_ON(rc > 0) and get the offending call trace leading to that problem. I suspect that one of the PHY drivers might be returning a positive value as part of a phy_read() call and that does not get properly filtered out. > > It might not be an ERRNO. E.g. https://lkml.org/lkml/2015/9/3/534 > fixed a bug where a positive value is returned which is not an > indication of an error. > > Andrew > -- Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
On Sat, Sep 05, 2015 at 09:47:55PM +0200, Andrew Lunn wrote: > On Sat, Sep 05, 2015 at 02:44:01PM -0500, Michael Welling wrote: > > On Sat, Sep 05, 2015 at 09:18:40PM +0200, Andrew Lunn wrote: > > > On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: > > > > The function phy_connect_direct can possibly return a positive > > > > return code. Using ERR_PTR with a positive value can lead to > > > > deferencing of an invalid pointer. > > > > > > Is this the correct fix? Would it not be better to find where the > > > positive return code is from and fix that? > > > > I guess I can trace it back to find out where the positive return code > > is originating. > > > > Is phy_connect_direct always supposed to return valid -errno? > > I would look at this from a different angle. A positive ERRNO is > probably a bug of some sort. So rather than papering over the cracks, > go find what the real issue is. > > It might not be an ERRNO. E.g. https://lkml.org/lkml/2015/9/3/534 > fixed a bug where a positive value is returned which is not an > indication of an error. It appears that I have stumbled upon the exact same issue as is fixed in the patch above. Good catch. > > Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
On Sat, Sep 05, 2015 at 01:11:51PM -0700, Florian Fainelli wrote: > Le 09/05/15 12:47, Andrew Lunn a écrit : > > On Sat, Sep 05, 2015 at 02:44:01PM -0500, Michael Welling wrote: > >> On Sat, Sep 05, 2015 at 09:18:40PM +0200, Andrew Lunn wrote: > >>> On Sat, Sep 05, 2015 at 01:01:29PM -0500, Michael Welling wrote: > The function phy_connect_direct can possibly return a positive > return code. Using ERR_PTR with a positive value can lead to > deferencing of an invalid pointer. > >>> > >>> Is this the correct fix? Would it not be better to find where the > >>> positive return code is from and fix that? > >> > >> I guess I can trace it back to find out where the positive return code > >> is originating. > >> > >> Is phy_connect_direct always supposed to return valid -errno? > > > > I would look at this from a different angle. A positive ERRNO is > > probably a bug of some sort. So rather than papering over the cracks, > > go find what the real issue is. > > Agreed, you could place a WARN_ON(rc > 0) and get the offending call > trace leading to that problem. I suspect that one of the PHY drivers > might be returning a positive value as part of a phy_read() call and > that does not get properly filtered out. > Thanks for the feedback. Does it hurt to always have a warning on positive return codes before using ERR_PTR? > > > > It might not be an ERRNO. E.g. https://lkml.org/lkml/2015/9/3/534 > > fixed a bug where a positive value is returned which is not an > > indication of an error. > > > >Andrew > > > -- > Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: Handle postive return codes in phy_connect
> > It might not be an ERRNO. E.g. https://lkml.org/lkml/2015/9/3/534 > > fixed a bug where a positive value is returned which is not an > > indication of an error. > > It appears that I have stumbled upon the exact same issue as is fixed in > the patch above. Ah, sorry. I put that bug there :-( The fix should be making its way into the kernel soon. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] net/ipv6: Correct PIM6 mrt_lock handling
From: Richard Laing Date: Thu, 3 Sep 2015 13:52:31 +1200 > In the IPv6 multicast routing code the mrt_lock was not being released > correctly in the MFC iterator, as a result adding or deleting a MIF would > cause a hang because the mrt_lock could not be acquired. > > This fix is a copy of the code for the IPv4 case and ensures that the lock > is released correctly. > > Signed-off-by: Richard Laing Applied and queued up for -stable, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: wan: sbni: fix device usage count
From: Sudip Mukherjee Date: Thu, 3 Sep 2015 11:30:30 +0530 > dev_get_by_name() will increment the usage count if the matching device > is found. But we were not decrementing the count if we have got the > device and the device is non-active. > > Signed-off-by: Sudip Mukherjee Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net] net: bridge: check __vlan_vid_del for error
Since __vlan_del can return an error code, change its inner function
__vlan_vid_del to return an eventual error from switchdev_port_obj_del.
Signed-off-by: Vivien Didelot
---
net/bridge/br_vlan.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 3cd8cc9..5f5a02b 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -117,10 +117,11 @@ out_filt:
return err;
}
-static void __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
- u16 vid)
+static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
+ u16 vid)
{
const struct net_device_ops *ops = dev->netdev_ops;
+ int err = 0;
/* If driver uses VLAN ndo ops, use 8021q to delete vid
* on device, otherwise try switchdev ops to delete vid.
@@ -137,8 +138,12 @@ static void __vlan_vid_del(struct net_device *dev, struct
net_bridge *br,
},
};
- switchdev_port_obj_del(dev, &vlan_obj);
+ err = switchdev_port_obj_del(dev, &vlan_obj);
+ if (err == -EOPNOTSUPP)
+ err = 0;
}
+
+ return err;
}
static int __vlan_del(struct net_port_vlans *v, u16 vid)
@@ -151,7 +156,11 @@ static int __vlan_del(struct net_port_vlans *v, u16 vid)
if (v->port_idx) {
struct net_bridge_port *p = v->parent.port;
- __vlan_vid_del(p->dev, p->br, vid);
+ int err;
+
+ err = __vlan_vid_del(p->dev, p->br, vid);
+ if (err)
+ return err;
}
clear_bit(vid, v->vlan_bitmap);
--
2.5.1
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: bridge: remove unnecessary switchdev include
Remove the unnecessary switchdev.h include from br_netlink.c. Signed-off-by: Vivien Didelot --- net/bridge/br_netlink.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index af5e187..ea748c9 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include "br_private.h" -- 2.5.1 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] 9p: trans_fd, initialize recv fcall properly if not set
On Thu, Sep 3, 2015 at 4:38 AM, Dominique Martinet wrote: > That code really should never be called (rc is allocated in > tag_alloc), but if it had been it couldn't have worked... > > Signed-off-by: Dominique Martinet > --- > net/9p/trans_fd.c | 3 +++ > 1 file changed, 3 insertions(+) > > To be honest, I think it might be better to just bail out if we get in > this switch (m->req->rc == NULL after p9_tag_lookup) and not try to > allocate more, because if we get there it's likely a race condition and > silently re-allocating will end up in more troubles than trying to > recover is worth. > Thoughts ? > Hmmm...trying to rattle my brain and remember why I put it in there back in 2008. It might have just been over-defensive programming -- or more likely it just pre-dated all the zero copy infrastructure which pretty much guaranteed we had an rc allocated and what is there is vestigial. I'm happy to accept a patch which makes this an assert, or perhaps just resets the connection because something has gone horribly wrong (similar to the ENOMEM path that is there now). -eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] Netfilter fixes for net
From: Pablo Neira Ayuso Date: Thu, 3 Sep 2015 11:50:55 +0200 > The following patchset contains Netfilter fixes for net, they are: > > 1) Oneliner to restore maps in nf_tables since we support addressing registers >at 32 bits level. > > 2) Restore previous default behaviour in bridge netfilter when CONFIG_IPV6=n, >oneliner from Bernhard Thaler. > > 3) Out of bound access in ipset hash:net* set types, reported by Dave Jones' >KASan utility, patch from Jozsef Kadlecsik. > > 4) Fix ipset compilation with gcc 4.4.7 related to C99 initialization of >unnamed unions, patch from Elad Raz. > > 5) Add a workaround to address inconsistent endianess in the res_id field of >nfnetlink batch messages, reported by Florian Westphal. > > 6) Fix error paths of CT/synproxy since the conntrack template was moved to > use >kmalloc, patch from Daniel Borkmann. > > All of them look good to me to reach 4.2, I can route this to -stable myself > too, just let me know what you prefer. > > You can pull these changes from: > > git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git Pulled, there was a merge conflict, please verify that I resolved it correctly. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [net-next PATCH] net: kill default_pref field of struct fib_rules_ops
From: Phil Sutter Date: Thu, 3 Sep 2015 13:10:22 +0200 > Since now all users of that field have been converted to use the generic > function fib_default_rule_pref() when assigning to it, fib_nl_newrule() > may just use it directly instead. > > Signed-off-by: Phil Sutter I did not instruct you to submit this change assuming I had applied your ipv6 one. Which I did not and will not do. Instead, I want you to do it all in one go. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] be2net: Revert "make the RX_FILTER command asynchronous" commit
From: Sathya Perla
Date: Thu, 3 Sep 2015 07:41:53 -0400
> The be_cmd_rx_filter() routine sends a non-embedded cmd to the FW and used
> a pre-allocated dma memory to hold the cmd payload. This worked fine when
> this cmd was synchronous. This cmd was changed to asynchronous mode by the
> commit 8af65c2f4("make the RX_FILTER command asynchronous"). So now when
> there are two quick invocations of this cmd, the 2nd request may end up
> overwriting the first request, causing FW cmd corruption.
>
> This patch reverts the offending commit and hence fixes the regression.
>
> Fixes: 8af65c2f4("be2net: make the RX_FILTER command asynchronous")
> Signed-off-by: Sathya Perla
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch net] switchdev: fix return value of switchdev_port_fdb_dump in case of error
From: Jiri Pirko
Date: Thu, 3 Sep 2015 14:04:17 +0200
> From: Jiri Pirko
>
> switchdev_port_fdb_dump is used as .ndo_fdb_dump. Its return value is
> idx, so we cannot return errval.
>
> Fixes: 45d4122ca7cd ("switchdev: add support for fdb add/del/dump via
> switchdev_port_obj ops.")
> Signed-off-by: Jiri Pirko
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: fec: normalize return value of pm_runtime_get_sync() in MDIO write
From: "Maciej S. Szmigiero" Date: Thu, 03 Sep 2015 21:38:30 +0200 > If fec MDIO write method succeeds its return value comes from > call to pm_runtime_get_sync(). > But pm_runtime_get_sync() can also return 1. > > In case of Micrel KSZ9031 PHY this value will then > be returned along the call chain of phy_write() -> > ksz9031_extended_write() -> ksz9031_center_flp_timing() -> > ksz9031_config_init() -> phy_init_hw() -> phy_attach_direct() -> > phy_connect_direct(). > > Then phy_connect() will cast it into a pointer using ERR_PTR(), > which then fec_enet_mii_probe() will try to dereference > resulting in an oops. > > Fix it by normalizing return value of pm_runtime_get_sync() > to be zero if positive in MDIO write method. > > Signed-off-by: Maciej Szmigiero Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next] RDS: rds_conn_lookup() should factor in the struct net for a match
From: Sowmini Varadhan
Date: Thu, 3 Sep 2015 16:24:52 -0400
>
> Only return a conn if the rds_conn_net(conn) matches the struct
> net passed to rds_conn_lookup().
>
> Fixes: 467fa15356ac ("RDS-TCP: Support multiple RDS-TCP listen endpoints,
>one per netns.")
>
> Signed-off-by: Sowmini Varadhan
Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: bridge: remove unnecessary switchdev include
Sun, Sep 06, 2015 at 03:49:41AM CEST, vivien.dide...@savoirfairelinux.com wrote: >Remove the unnecessary switchdev.h include from br_netlink.c. > >Signed-off-by: Vivien Didelot Acked-by: Jiri Pirko -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net] net: bridge: check __vlan_vid_del for error
Sun, Sep 06, 2015 at 03:27:57AM CEST, vivien.dide...@savoirfairelinux.com wrote: >Since __vlan_del can return an error code, change its inner function >__vlan_vid_del to return an eventual error from switchdev_port_obj_del. > >Signed-off-by: Vivien Didelot Acked-by: Jiri Pirko -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V9fs-developer] [PATCH] 9p: trans_fd, initialize recv fcall properly if not set
Eric Van Hensbergen wrote on Sat, Sep 05, 2015: > On Thu, Sep 3, 2015 at 4:38 AM, Dominique Martinet > wrote: > > To be honest, I think it might be better to just bail out if we get in > > this switch (m->req->rc == NULL after p9_tag_lookup) and not try to > > allocate more, because if we get there it's likely a race condition and > > silently re-allocating will end up in more troubles than trying to > > recover is worth. > > Thoughts ? > > > > Hmmm...trying to rattle my brain and remember why I put it in there > back in 2008. > It might have just been over-defensive programming -- or more likely it just > pre-dated all the zero copy infrastructure which pretty much guaranteed we had > an rc allocated and what is there is vestigial. I'm happy to accept a > patch which > makes this an assert, or perhaps just resets the connection because something > has gone horribly wrong (similar to the ENOMEM path that is there now). Yeah, it looks like the safety comes from the zero-copy stuff that came much later. Let's go with resetting the connection then. Hmm. EIO is a bit too generic so would be good to avoid that if possible, but can't think of anything better... Speaking of zero-copy, I believe it should be fairly straight-forward to implement for trans_fd now I've actually looked at it, since we do the payload read after a p9_tag_lookup, would just need m->req to point to a zc buffer. Write is similar, if there's a zc buffer just send it after the header. The cost is a couple more pointers in req and an extra if in both workers, that seems pretty reasonable. Well, I'm not using trans_fd much here (and unfortunately zero-copy isn't possible at all given the transport protocol for RDMA, at least for recv), but if anyone cares it probably could be done without too much hassle for the fd workers. -- Dominique -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
