* RE: [PATCH 1/1] update ip-sysctl.txt documentation
From: David Laight @ 2015-01-07 10:11 UTC (permalink / raw)
To: 'Ani Sinha', corbet@lwn.net, davem@davemloft.net,
edumazet@google.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, P@draigbrady.com,
netdev@vger.kernel.org, fruggeri@arista.com
In-Reply-To: <1420587382-24612-1-git-send-email-ani@arista.com>
From: Ani Sinha
> Update documentation to reflect the fact that
> /proc/sys/net/ipv4/route/max_size is no longer used for ipv4.
>
> Signed-off-by: Ani Sinha <ani@arista.com>
> ---
> Documentation/networking/ip-sysctl.txt | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 9bffdfc..2a28261 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -64,8 +64,10 @@ fwmark_reflect - BOOLEAN
> Default: 0
>
> route/max_size - INTEGER
> - Maximum number of routes allowed in the kernel. Increase
> - this when using large numbers of interfaces and/or routes.
> + Post linux kernel 3.6, this is deprecated for ipv4 as route cache is no
> + longer used. For ipv6, this is used to limit the maximum number of ipv6
> + routes allowed in the kernel. Increase this when using large numbers of
> + interfaces and/or routes.
Now imagine you are reading ip-sysctl.txt, the new text is all wrong.
Something like:
Limit on the size of the ip route caches.
Ignored for ipv4 after kernel 3.6 because the ipv4 route cache was removed.
Increase this when using large numbers of interfaces and/or routes.
would read better.
David
^ permalink raw reply
* Re: [net-next PATCH v1 01/11] net: flow_table: create interface for hw match/action tables
From: Or Gerlitz @ 2015-01-07 10:07 UTC (permalink / raw)
To: John Fastabend
Cc: Thomas Graf, Scott Feldman, Jiří Pírko,
Jamal Hadi Salim, simon.horman, Linux Netdev List, David Miller,
Andy Gospodarek
In-Reply-To: <54AADEFF.3090306@gmail.com>
On Mon, Jan 5, 2015 at 8:59 PM, John Fastabend <john.fastabend@gmail.com> wrote:
>>> +struct sk_buff *net_flow_build_actions_msg(struct net_flow_action **a,
>>> + struct net_device *dev,
>>> + u32 portid, int seq, u8 cmd)
>>> +{
>>> + struct genlmsghdr *hdr;
>>> + struct sk_buff *skb;
>>> + int err = -ENOBUFS;
>>> +
>>> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>
>>
>> genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
>
>
> fixed along with the other cases.
small nit here, net_flow_build_actions_msg can be made static, it's
called only from within this file
few more nits... checkpatch --strict produces bunch of "CHECK: Please
use a blank line after function/struct/union/enum declarations"
comments, I guess worth fixing too.
^ permalink raw reply
* Re: [RFC PATCH] unlock rtnl mutex in ic_open_devs while waiting
From: Maarten Lankhorst @ 2015-01-07 10:06 UTC (permalink / raw)
To: David Miller
Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel,
Jay Cliburn, Chris Snook, Sabrina Dubroca, Johannes Berg
In-Reply-To: <20150106.172152.2034122593693302134.davem@davemloft.net>
Op 06-01-15 om 23:21 schreef David Miller:
> From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> Date: Mon, 05 Jan 2015 14:52:06 +0100
>
>> This fixes a deadlock with alx_link_check, which takes the rtnl_mutex in
>> a work item to check the link.
>>
>> I have no idea whether alx should be fixed or ipconfig.c,
>> but this saves 120 seconds off my boot time. ;-)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> I genuinely think that alx_link_check() needs to use a smaller hammer
> to do it's locking, there is no reason to use the RTNL mutex.
>
> A driver private mutex will probably work just as well and not have
> this problem.
I guess alx_check_link uses the rtnl_lock for serializing against any possible alx_reset call.
The alternative is stopping check_link work before running anything that changes the device state.
Does the below patch look sane instead?
---
diff --git a/drivers/net/ethernet/atheros/alx/alx.h b/drivers/net/ethernet/atheros/alx/alx.h
index 8fc93c5f6abc..354f155b3144 100644
--- a/drivers/net/ethernet/atheros/alx/alx.h
+++ b/drivers/net/ethernet/atheros/alx/alx.h
@@ -88,6 +88,8 @@ struct alx_priv {
unsigned int size;
} descmem;
+ bool stop_link_check;
+
/* protect int_mask updates */
spinlock_t irq_lock;
u32 int_mask;
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index e398eda07298..ae93b8052cbf 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -315,7 +316,8 @@ static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
*/
alx->int_mask &= ~ALX_ISR_PHY;
write_int_mask = true;
- alx_schedule_link_check(alx);
+ if (!alx->stop_link_check)
+ alx_schedule_link_check(alx);
}
if (intr & (ALX_ISR_TX_Q0 | ALX_ISR_RX_Q0)) {
@@ -742,6 +744,17 @@ static netdev_features_t alx_fix_features(struct net_device *netdev,
return features;
}
+static void alx_disable_link_check(struct alx_priv *alx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&alx->irq_lock, flags);
+ alx->stop_link_check = true;
+ spin_unlock_irqrestore(&alx->irq_lock, flags);
+
+ cancel_work_sync(&alx->link_check_wk);
+}
+
static void alx_netif_stop(struct alx_priv *alx)
{
alx->dev->trans_start = jiffies;
@@ -756,6 +769,7 @@ static void alx_halt(struct alx_priv *alx)
{
struct alx_hw *hw = &alx->hw;
+ alx_disable_link_check(alx);
alx_netif_stop(alx);
hw->link_speed = SPEED_UNKNOWN;
hw->duplex = DUPLEX_UNKNOWN;
@@ -788,6 +802,7 @@ static void alx_activate(struct alx_priv *alx)
/* clear old interrupts */
alx_write_mem32(&alx->hw, ALX_ISR, ~(u32)ALX_ISR_DIS);
+ alx->stop_link_check = false;
alx_irq_enable(alx);
alx_schedule_link_check(alx);
@@ -850,6 +865,7 @@ static int __alx_open(struct alx_priv *alx, bool resume)
/* clear old interrupts */
alx_write_mem32(&alx->hw, ALX_ISR, ~(u32)ALX_ISR_DIS);
+ alx->stop_link_check = false;
alx_irq_enable(alx);
if (!resume)
@@ -966,9 +982,7 @@ static void alx_link_check(struct work_struct *work)
alx = container_of(work, struct alx_priv, link_check_wk);
- rtnl_lock();
alx_check_link(alx);
- rtnl_unlock();
}
^ permalink raw reply related
* RE: [PATCH 2/6] vxlan: Group Policy extension
From: David Laight @ 2015-01-07 10:03 UTC (permalink / raw)
To: 'Alexei Starovoitov', Thomas Graf
Cc: David S. Miller, Jesse Gross, Stephen Hemminger, Pravin Shelar,
netdev@vger.kernel.org, dev@openvswitch.org
In-Reply-To: <CAADnVQ+EO34qYbK+sji-Da3CMkn+-V-9Uvdar_WgxRQ4_+JFXA@mail.gmail.com>
From: Alexei Starovoitov
> On Tue, Jan 6, 2015 at 6:05 PM, Thomas Graf <tgraf@suug.ch> wrote:
> > +struct vxlan_gbp {
> > +#ifdef __LITTLE_ENDIAN_BITFIELD
> > + __u8 reserved_flags1:3,
> ...
> > + __be16 policy_id;
> > +} __packed;
>
> are you sure that compiler will be smart enough
> to do single short load when you pack
> u8 + struct vxlan_gbp inside struct vxlanhdr ?
> I suspect compiler will use two byte loads
> with shifts and ors every time to access policy_id.
> Even it works ok, I think this struct layout is ugly.
> imo would be much easier to read if you replace
> the whole vxlanhdr with vxlanhdr_gbp
> or split vxlanhdr into two 32-bit structs.
> then __packed hacks won't be needed.
Also, if you are writing the values then you need to write
all the members of the bitfield in order to get a single write.
Basically you are much better off using explicit masks.
David
^ permalink raw reply
* Re: [PATCH net-next v2 0/7] Involve rhashtable_lookup_insert routine
From: Thomas Graf @ 2015-01-07 9:54 UTC (permalink / raw)
To: Ying Xue
Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-1-git-send-email-ying.xue@windriver.com>
On 01/07/15 at 01:41pm, Ying Xue wrote:
> The series aims to involve rhashtable_lookup_insert() to guarantee
> that the process of lookup and insertion of an object from/into hash
> table is finished atomically, allowing rhashtable's users not to
> introduce an extra lock during search and insertion. For example,
> tipc socket is the first user benefiting from this enhancement.
>
> v2 changes:
Thanks, this looks excellent. I think patch 6/7 can be dropped,
otherwise this looks ready to go. I acked all other changes.
^ permalink raw reply
* Re: [PATCH net-next v2 6/7] rhashtable: initialize atomic nelems variable
From: Thomas Graf @ 2015-01-07 9:53 UTC (permalink / raw)
To: Ying Xue
Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-7-git-send-email-ying.xue@windriver.com>
On 01/07/15 at 01:41pm, Ying Xue wrote:
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> Cc: Thomas Graf <tgraf@suug.ch>
Is this really needed at all? We initialize the full rhashtable
struct to 0 in rhashtable_init().
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] rhashtable: avoid unnecessary wakeup for worker queue
From: Thomas Graf @ 2015-01-07 9:50 UTC (permalink / raw)
To: Ying Xue
Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-6-git-send-email-ying.xue@windriver.com>
On 01/07/15 at 01:41pm, Ying Xue wrote:
> Move condition statements of verifying whether hash table size exceeds
> its maximum threshold or reaches its minimum threshold from resizing
> functions to resizing decision functions, avoiding unnecessary wakeup
> for worker queue thread.
>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> Cc: Thomas Graf <tgraf@suug.ch>
Good optimization, thanks!
Acked-by: Thomas Graf <tgraf@suug.ch>
Can you do a follow-up patch and add a note in rhashtable.h to
indicate the implementation of the grow and shrink decision function
must enforce min/max shift?
^ permalink raw reply
* [PATCH -next] r8169: add support for xmit_more
From: Florian Westphal @ 2015-01-07 9:49 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Delay update of hw tail descriptor if we know that another skb is going
to be sent.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
drivers/net/ethernet/realtek/r8169.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 14a1c5c..3a28059 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7049,6 +7049,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
u32 status, len;
u32 opts[2];
int frags;
+ bool stop_queue;
if (unlikely(!TX_FRAGS_READY_FOR(tp, skb_shinfo(skb)->nr_frags))) {
netif_err(tp, drv, dev, "BUG! Tx Ring full when queue awake!\n");
@@ -7105,11 +7106,16 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
tp->cur_tx += frags + 1;
- RTL_W8(TxPoll, NPQ);
+ stop_queue = !TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS);
- mmiowb();
+ if (!skb->xmit_more || stop_queue ||
+ netif_xmit_stopped(netdev_get_tx_queue(dev, 0))) {
+ RTL_W8(TxPoll, NPQ);
+
+ mmiowb();
+ }
- if (!TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS)) {
+ if (stop_queue) {
/* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
* not miss a ring update when it notices a stopped queue.
*/
--
2.0.5
^ permalink raw reply related
* [PATCH next] net: e1000e: support txtd update delay via xmit_more
From: Florian Westphal @ 2015-01-07 9:49 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Don't update tx tail descriptor if queue hasn't been stopped
and we know at least one more skb will be sent right away.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 332a298..214cb78 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5444,16 +5444,6 @@ static void e1000_tx_queue(struct e1000_ring *tx_ring, int tx_flags, int count)
wmb();
tx_ring->next_to_use = i;
-
- if (adapter->flags2 & FLAG2_PCIM2PCI_ARBITER_WA)
- e1000e_update_tdt_wa(tx_ring, i);
- else
- writel(i, tx_ring->tail);
-
- /* we need this if more than one processor can write to our tail
- * at a time, it synchronizes IO on IA64/Altix systems
- */
- mmiowb();
}
#define MINIMUM_DHCP_PACKET_SIZE 282
@@ -5653,6 +5643,19 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
(MAX_SKB_FRAGS *
DIV_ROUND_UP(PAGE_SIZE,
adapter->tx_fifo_limit) + 2));
+
+ if (!skb->xmit_more ||
+ netif_xmit_stopped(netdev_get_tx_queue(netdev, 0))) {
+ if (adapter->flags2 & FLAG2_PCIM2PCI_ARBITER_WA)
+ e1000e_update_tdt_wa(tx_ring, tx_ring->next_to_use);
+ else
+ writel(tx_ring->next_to_use, tx_ring->tail);
+
+ /* we need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
+ }
} else {
dev_kfree_skb_any(skb);
tx_ring->buffer_info[first].time_stamp = 0;
--
2.0.5
^ permalink raw reply related
* Re: [PATCH net-next v2 4/7] rhashtable: future table needs to be traversed when remove an object
From: Thomas Graf @ 2015-01-07 9:39 UTC (permalink / raw)
To: Ying Xue
Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-5-git-send-email-ying.xue@windriver.com>
On 01/07/15 at 01:41pm, Ying Xue wrote:
> When remove an object from hash table, we currently only traverse old
> bucket table to check whether the object exists. If the object is not
> found in it, we will try again. But in the second search loop, we still
> search the object from the old table instead of future table. As a
> result, the object may be not removed from hash table especially when
> resizing is currently in progress and the object is just saved in the
> future table.
>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> Cc: Thomas Graf <tgraf@suug.ch>
Nice catch!
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: [PATCH net-next v2 2/7] rhashtable: introduce rhashtable_wakeup_worker helper function
From: Thomas Graf @ 2015-01-07 9:34 UTC (permalink / raw)
To: Ying Xue
Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-3-git-send-email-ying.xue@windriver.com>
On 01/07/15 at 01:41pm, Ying Xue wrote:
> Introduce rhashtable_wakeup_worker() helper function to reduce
> duplicated code where to wake up worker.
>
> By the way, as long as the both "future_tbl" and "tbl" bucket table
> pointers point to the same bucket array, we should try to wake up
> the resizing worker thread, otherwise, it indicates the work of
> resizing hash table is not finished yet. However, currently we will
> wake up the worker thread only when the two pointers point to
> different bucket array. Obviously this is wrong. So, the issue is
> also fixed as well in the patch.
>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> Cc: Thomas Graf <tgraf@suug.ch>
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: [PATCH 6/6] driver/net/irda: Replace timeval with ktime_t in vlsi_ir
From: Arnd Bergmann @ 2015-01-07 8:59 UTC (permalink / raw)
To: Chunyan Zhang; +Cc: samuel, zhang.lyra, netdev, linux-kernel
In-Reply-To: <1420601978-15866-7-git-send-email-zhang.chunyan@linaro.org>
On Wednesday 07 January 2015 11:39:38 Chunyan Zhang wrote:
> This patch changes the 32-bit time type (timeval) to the 64-bit one
> (ktime_t), since 32-bit time types will break in the year 2038.
> So, I use ktime_t instead of all uses of timeval.
>
> This patch also changes do_gettimeofday() to ktime_get() accordingly,
> since ktime_get returns a ktime_t, but do_gettimeofday returns a
> struct timeval, and the other reason is that ktime_get() uses
> the monotonic clock.
>
> This patch uses ktime_us_delta to get the elapsed time of microsecond,
> and uses div_s64_rem to get what seconds & microseconds time elapsed
> for printing.
>
> This patch also changes the code of function 'vlsi_hard_start_xmit' to
> do the same things as the others drivers, that is passing the remaining
> time into udelay() instead of looping until enough time has passed.
>
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
All six patches look correct to me now, nice work!
I have a few very minor comments still, some of which apply to the
other patches as well:
* Your patch changelog starts each paragraph with 'This patch', which is
correct but is a little strange to read. You could start the first paragraph
explaining what the driver currently does and why we want to change it,
like "The vlsi ir driver uses 'timeval', which we try to remove in the
kernel because all 32-bit time types will break in the year 2038".
* It would also be good to explain that none of these drivers are actually
broken regarding y2038 at the moment and that the change is only done to
remove the need for auditing this fact again.
> @@ -180,8 +181,8 @@ static void vlsi_proc_ndev(struct seq_file *seq, struct net_device *ndev)
> vlsi_irda_dev_t *idev = netdev_priv(ndev);
> u8 byte;
> u16 word;
> - unsigned delta1, delta2;
> - struct timeval now;
> + ktime_t now;
> + s32 sec, usec;
> unsigned iobase = ndev->base_addr;
* you now have a local variable that is only used to be passed into ktime_us_delta.
While there is no difference to the compiler, I would probably shorten this
and avoid the local variable by passing the result of ktime_get() directly
into ktime_us_delta().
For the drivers that store the 'now' variable in a data structure, doing the
same change shrinks the driver specific data structure, which is an added
benefit.
> - for(;;) {
> - do_gettimeofday(&now);
> - if (now.tv_sec > ready.tv_sec ||
> - (now.tv_sec==ready.tv_sec && now.tv_usec>=ready.tv_usec))
> - break;
> - udelay(100);
> - /* must not sleep here - called under netif_tx_lock! */
> - }
> + now = ktime_get();
> + diff = ktime_us_delta(now, idev->last_rx);
> + if (mtt > diff)
> + udelay(mtt - diff);
> }
The change looks good, but you remove a useful comment about sleeping inside of
netif_tx_lock. I would not bother adding the same comment to the other drivers,
but it still applies to the udelay call here so I would move it there.
Note that generally we try hard to avoid the use of long 'udelay' calls, but
I wouldn't expect you to change the drivers for this.
When you submit the next version and you have addressed my last comments,
please add
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Arnd
^ permalink raw reply
* Re: [PATCH] brcm80211: brcmsmac: dma: Remove some unused functions
From: Arend van Spriel @ 2015-01-07 8:58 UTC (permalink / raw)
To: Julia Lawall
Cc: Rickard Strandqvist, Kalle Valo, Larry Finger, Brett Rudley,
Hante Meuleman, Fabian Frederick, linux-wireless@vger.kernel.org,
brcm80211-dev-list, Network Development,
Linux Kernel Mailing List
In-Reply-To: <alpine.DEB.2.02.1501070726320.2058@localhost6.localdomain6>
On 01/07/15 07:29, Julia Lawall wrote:
>
>
> On Wed, 7 Jan 2015, Rickard Strandqvist wrote:
>
>> 2015-01-05 12:06 GMT+01:00 Arend van Spriel<arend@broadcom.com>:
>>> On 01/05/15 11:49, Kalle Valo wrote:
>>>>
>>>> Rickard Strandqvist<rickard_strandqvist@spectrumdigital.se> writes:
>>>>
>>>>> As I hope you can see I have made some changes regarding the
>>>>> subject-line. Thought it was an advantage to be able to see which file
>>>>> I actually removed something from. There seems to be a big focus on
>>>>> getting right on subject-line right in recent weeks.
>>>>>
>>>>> I wonder why there is a script that takes a file name, and respond
>>>>> with an appropriate subject line?
>>>
>>>
>>> Is there a script for this? Anyway, I would say driver name is enough.
>>> Enough about the subject line ;-) I would like to give some general remarks
>>> as you seem to touch a lot of kernel code. First off, I think it is good to
>>> remove unused stuff. However, I would like some more explanation on your
>>> methodology apart from "partially found by using a static code analysis
>>> program". So a cover-letter explaining that would have been nice (maybe
>>> still is). Things like Kconfig option can affect whether function are used
>>> or not so how did you cover that.
>>>
>>> Regards,
>>> Arend
>>>
>>>
>>>> I don't think you can really automate this as some drivers do this a bit
>>>> differently. You always need to manually check the commit log.
>>>>
>>>>> But ok, I change my script accordingly. Should I submit the patch again?
>>>>
>>>>
>>>> Yes, please resubmit.
>>>>
>>>
>>
>> Hi Arend
>>
>> Yes, a script that had been excellent, I think!
>> I have one as part of my git send-email script, until a week ago, it
>> was enough that I removed the "drivers/" and changed all "/" to ": "
>> I have now been expanded my sed pipe a lot (tell me if anyone is interested)
>> But now I've seen everything from uppercase and [DIR], etc.
>> So I can not understand how anyone should be able to get the right
>> name without a good help.
>>
>> Sure i like to share how I use cppcheck, but is very hesitant to write
>> this with each patch mails I send though!
>>
>> I run:
>> cppcheck --force --quiet --enable=all .
>>
>> Or a specific file instead of .
>>
>> This will include, among other things get a lot of error message such,
>> +4000 for the kernel.
>> (style) The function 'xxx' is never used
>>
>> For these I made a script that searched through all the files after
>> the function name (cppcheck missed a few). And save the rest so I go
>> through them and possibly send patches.
>
> I think that the question was about what methodology is cppcheck using to
> find the given issue. But probably cppcheck is a black box that does
> whatever it does, so the user doesn't know what the rationale is.
That would have been nice, but I also wanted to know what his subsequent
steps were to validate the output from cppcheck. I went through some
cppcheck web pages, but they only elaborate on what is can do and not
the how. But hey, it is an open-source tool so there is always the code
to check.
> However, I think you mentioned that cppcheck found only some of the
> issues. You could thus describe what was the methodology for finding the
> other ones.
Maybe upon removing an unused function it had a ripple effect on others
becoming unused as well? Still this is speculating and with this kind of
cleanup effort all over the place it is better to review the methodology.
Regards,
Arend
> julia
^ permalink raw reply
* Re: [PATCH] brcm80211: brcmsmac: dma: Remove some unused functions
From: Arend van Spriel @ 2015-01-07 8:57 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Kalle Valo, Larry Finger, Brett Rudley, Hante Meuleman,
Fabian Frederick, linux-wireless@vger.kernel.org,
brcm80211-dev-list, Network Development,
Linux Kernel Mailing List, Julia Lawall
In-Reply-To: <CAKXHbyNBV6=DPF4n0onzhEqAuS-PqLS5pwZj_OitRuYMvgpjvw@mail.gmail.com>
On 01/07/15 00:33, Rickard Strandqvist wrote:
> 2015-01-05 12:06 GMT+01:00 Arend van Spriel<arend@broadcom.com>:
>> On 01/05/15 11:49, Kalle Valo wrote:
>>>
>>> Rickard Strandqvist<rickard_strandqvist@spectrumdigital.se> writes:
>>>
>>>> As I hope you can see I have made some changes regarding the
>>>> subject-line. Thought it was an advantage to be able to see which file
>>>> I actually removed something from. There seems to be a big focus on
>>>> getting right on subject-line right in recent weeks.
>>>>
>>>> I wonder why there is a script that takes a file name, and respond
>>>> with an appropriate subject line?
>>
>>
>> Is there a script for this? Anyway, I would say driver name is enough.
>> Enough about the subject line ;-) I would like to give some general remarks
>> as you seem to touch a lot of kernel code. First off, I think it is good to
>> remove unused stuff. However, I would like some more explanation on your
>> methodology apart from "partially found by using a static code analysis
>> program". So a cover-letter explaining that would have been nice (maybe
>> still is). Things like Kconfig option can affect whether function are used
>> or not so how did you cover that.
>>
>> Regards,
>> Arend
>>
>>
>>> I don't think you can really automate this as some drivers do this a bit
>>> differently. You always need to manually check the commit log.
>>>
>>>> But ok, I change my script accordingly. Should I submit the patch again?
>>>
>>>
>>> Yes, please resubmit.
>>>
>>
>
> Hi Arend
>
> Yes, a script that had been excellent, I think!
> I have one as part of my git send-email script, until a week ago, it
> was enough that I removed the "drivers/" and changed all "/" to ": "
> I have now been expanded my sed pipe a lot (tell me if anyone is interested)
> But now I've seen everything from uppercase and [DIR], etc.
> So I can not understand how anyone should be able to get the right
> name without a good help.
>
> Sure i like to share how I use cppcheck, but is very hesitant to write
> this with each patch mails I send though!
>
> I run:
> cppcheck --force --quiet --enable=all .
And . is the top-level directory in the kernel repo? I am not familiar
with cppcheck, but does it invoke the kernel Makefile. From a quick
glance on cppcheck webpage I guess you could enable only the unused
function checker.
> Or a specific file instead of .
>
> This will include, among other things get a lot of error message such,
> +4000 for the kernel.
> (style) The function 'xxx' is never used
>
> For these I made a script that searched through all the files after
> the function name (cppcheck missed a few). And save the rest so I go
> through them and possibly send patches.
All the file? Within the same driver or kernel-wide. So now "go through
them" means compile testing with applicable Kconfig selections?
Gr. AvS
^ permalink raw reply
* Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
From: Michael S. Tsirkin @ 2015-01-07 8:57 UTC (permalink / raw)
To: Greg Kurz
Cc: Alex Williamson, thuth, kvm, rusty, netdev, linux-kernel,
virtualization, dahi, pbonzini, David Miller
In-Reply-To: <20150107093105.6ef918f1@bahia.local>
On Wed, Jan 07, 2015 at 09:31:05AM +0100, Greg Kurz wrote:
> On Tue, 06 Jan 2015 16:55:30 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
> > On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote:
> > > I had to add an explicit tag to suppress compiler warning:
> > > gcc isn't smart enough to notice that
> > > len is always initialized since function is called with size > 0.
> >
> > I'm getting a panic inside a guest when this change is applied on the
> > host. I identified this patch via bisect and confirmed by reverting it
> > from v3.19-rc2. Guest is centos6. Thanks,
> >
> > Alex
> >
> > commit 8b38694a2dc8b18374310df50174f1e4376d6824
> > Author: Michael S. Tsirkin <mst@redhat.com>
> > Date: Fri Oct 24 14:19:48 2014 +0300
> >
> > vhost/net: virtio 1.0 byte swap
> >
> > I had to add an explicit tag to suppress compiler warning:
> > gcc isn't smart enough to notice that
> > len is always initialized since function is called with size > 0.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >
>
> This chunk looks suspicious:
>
> - heads[headcount - 1].len += datalen;
> + heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
>
> s/len - datalen/len + datalen/ ?
Indeed!
I just sent a patch fixing this, thanks a lot.
> > XML chunk:
> >
> > <interface type='direct'>
> > <mac address='52:54:00:64:f3:34'/>
> > <source dev='iscsinet0' mode='bridge'/>
> > <model type='virtio'/>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> > </interface>
> >
> > Panic log:
> >
> > <1>BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> > <1>IP: [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> > <4>PGD 1aa2f4067 PUD 1aa2f5067 PMD 0
> > <4>Oops: 0000 [#1] SMP
> > <4>last sysfs file: /sys/devices/pci0000:00/0000:00:03.0/virtio0/net/eth9/ifindex
> > <4>CPU 0
> > <4>Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last unloaded: speedstep_lib]
> > <4>
> > <4>Pid: 1374, comm: NetworkManager Tainted: P --------------- 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 1996)
> > <4>RIP: 0010:[<ffffffffa0079469>] [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> > <4>RSP: 0018:ffff880028203e48 EFLAGS: 00010246
> > <4>RAX: ffff8801a3383d00 RBX: ffff8801a6aaf480 RCX: ffff8801aa20b6e0
> > <4>RDX: 00000000000000c0 RSI: ffff8801a3383c00 RDI: ffff8801a3383cc0
> > <4>RBP: ffff880028203ed8 R08: 000000000000009e R09: ffff8801aa1d800c
> > <4>R10: 0000000000000218 R11: 0000000000000000 R12: ffff8801aa20b6e0
> > <4>R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> > <4>FS: 00007febf114d800(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
> > <4>CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > <4>CR2: 0000000000000010 CR3: 00000001aa793000 CR4: 00000000000006f0
> > <4>DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > <4>DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > <4>Process NetworkManager (pid: 1374, threadinfo ffff8801a74ba000, task ffff8801a8d56040)
> > <4>Stack:
> > <4> ffff8801aa1d8000 000000000000009e ffff8801aa20b6e0 ffff8801aa20b718
> > <4><d> ffff8801aa20b780 ffff8801aa1d800c ffff8801a6aaf4b8 ffff8801aa20b020
> > <4><d> 0000000000000080 ffff8801aa20b708 0000000000000001 00001f5981a830c8
> > <4>Call Trace:
> > <4> <IRQ>
> > <4> [<ffffffff8146ae33>] net_rx_action+0x103/0x2f0
> > <4> [<ffffffff8107a5f1>] __do_softirq+0xc1/0x1e0
> > <4> [<ffffffff8100c30c>] ? call_softirq+0x1c/0x30
> > <4> [<ffffffff8100c30c>] call_softirq+0x1c/0x30
> > <4> <EOI>
> > <4> [<ffffffff8100fa75>] ? do_softirq+0x65/0xa0
> > <4> [<ffffffff8107b2ea>] local_bh_enable+0x9a/0xb0
> > <4> [<ffffffffa007813a>] virtnet_napi_enable+0x4a/0x60 [virtio_net]
> > <4> [<ffffffffa0078ebf>] virtnet_open+0x4f/0x60 [virtio_net]
> > <4> [<ffffffff81467691>] dev_open+0xa1/0x100
> > <4> [<ffffffff81466751>] dev_change_flags+0xa1/0x1d0
> > <4> [<ffffffff81474a59>] do_setlink+0x169/0x8b0
> > <4> [<ffffffff814770b6>] ? rtnl_fill_ifinfo+0x946/0xcb0
> > <4> [<ffffffff812a3d24>] ? nla_parse+0x34/0x110
> > <4> [<ffffffff8147659e>] rtnl_setlink+0xee/0x130
> > <4> [<ffffffff81475b67>] rtnetlink_rcv_msg+0x2d7/0x340
> > <4> [<ffffffff81231e14>] ? socket_has_perm+0x74/0x90
> > <4> [<ffffffff81475890>] ? rtnetlink_rcv_msg+0x0/0x340
> > <4> [<ffffffff814910a9>] netlink_rcv_skb+0xa9/0xd0
> > <4> [<ffffffff81475875>] rtnetlink_rcv+0x25/0x40
> > <4> [<ffffffff81490cdb>] netlink_unicast+0x2db/0x320
> > <4> [<ffffffff81491750>] netlink_sendmsg+0x2c0/0x3d0
> > <4> [<ffffffff814520c3>] sock_sendmsg+0x123/0x150
> > <4> [<ffffffff81453d73>] ? sock_recvmsg+0x133/0x160
> > <4> [<ffffffff8109afa0>] ? autoremove_wake_function+0x0/0x40
> > <4> [<ffffffff81136941>] ? lru_cache_add_lru+0x21/0x40
> > <4> [<ffffffff8115522d>] ? page_add_new_anon_rmap+0x9d/0xf0
> > <4> [<ffffffff8114aeef>] ? handle_pte_fault+0x4af/0xb00
> > <4> [<ffffffff81451f14>] ? move_addr_to_kernel+0x64/0x70
> > <4> [<ffffffff814538b6>] __sys_sendmsg+0x406/0x420
> > <4> [<ffffffff8104a98c>] ? __do_page_fault+0x1ec/0x480
> > <4> [<ffffffff814523d9>] ? sys_sendto+0x139/0x190
> > <4> [<ffffffff8103ea6c>] ? kvm_clock_read+0x1c/0x20
> > <4> [<ffffffff81453ad9>] sys_sendmsg+0x49/0x90
> > <4> [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b
> > <4>Code: 83 e0 00 00 00 00 10 00 00 48 03 93 d0 00 00 00 66 83 42 04 01 8b 93 cc 00 00 00 48 8b b3 d0 00 00 00 80 4c 16 10 20 44 2b 68 0c <4d> 8b 76 10 75 89 e9 d1 fd ff ff 0f 1f 40 00 a8 02 74 0d 0f b6
> > <1>RIP [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> > <4> RSP <ffff880028203e48>
> > <4>CR2: 0000000000000010
> >
> >
> > _______________________________________________
> > Virtualization mailing list
> > Virtualization@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/virtualization
> >
^ permalink raw reply
* [PATCH] vhost/net: length miscalculation
From: Michael S. Tsirkin @ 2015-01-07 8:55 UTC (permalink / raw)
To: linux-kernel; +Cc: Alex Williamson, Greg Kurz, kvm, virtualization, netdev
commit 8b38694a2dc8b18374310df50174f1e4376d6824
vhost/net: virtio 1.0 byte swap
had this chunk:
- heads[headcount - 1].len += datalen;
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
This adds datalen with the wrong sign, causing guest panics.
Fixes: 8b38694a2dc8b18374310df50174f1e4376d6824
Reported-by: Alex Williamson <alex.williamson@redhat.com>
Suggested-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Alex, could you please confirm this fixes the crash for you?
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 14419a8..d415d69 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -538,7 +538,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
++headcount;
seg += in;
}
- heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
*iovcount = seg;
if (unlikely(log))
*log_num = nlogs;
--
MST
^ permalink raw reply related
* Re: [PATCH] sh_eth: Fix access to TRSCER register
From: Geert Uytterhoeven @ 2015-01-07 8:42 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: netdev@vger.kernel.org, Yoshihiro Shimoda, Linux-sh list
In-Reply-To: <1420608947-6861-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com>
Hi Iwamatsu-san,
On Wed, Jan 7, 2015 at 6:35 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> TRSCER register is configured differently by SoCs. TRSCER of R-Car is
> RINT8 bit only valid, other bits are reserved bits.
> This removes access to TRSCER register reserve bit.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index c29ba80..59ee457 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1294,7 +1294,11 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
> /* Frame recv control (enable multiple-packets per rx irq) */
> sh_eth_write(ndev, RMCR_RNC, RMCR);
>
> - sh_eth_write(ndev, DESC_I_RINT8 | DESC_I_RINT5 | DESC_I_TINT2, TRSCER);
> + if (mdp->cd->register_type == SH_ETH_REG_FAST_RCAR)
This catches both the R-Car Gen1 and R-Car Gen2 cases.
According to the datasheets, r8a7778/9 do have the other bits?
Only R-Car Gen2 doesn't have them.
> + sh_eth_write(ndev, DESC_I_RINT8, TRSCER);
> + else
> + sh_eth_write(ndev, DESC_I_RINT8 | DESC_I_RINT5 | DESC_I_TINT2,
> + TRSCER);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
From: Greg Kurz @ 2015-01-07 8:31 UTC (permalink / raw)
To: Alex Williamson
Cc: thuth, kvm, Michael S. Tsirkin, rusty, netdev, linux-kernel,
virtualization, dahi, pbonzini, David Miller
In-Reply-To: <1420588530.3541.116.camel@redhat.com>
On Tue, 06 Jan 2015 16:55:30 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote:
> > I had to add an explicit tag to suppress compiler warning:
> > gcc isn't smart enough to notice that
> > len is always initialized since function is called with size > 0.
>
> I'm getting a panic inside a guest when this change is applied on the
> host. I identified this patch via bisect and confirmed by reverting it
> from v3.19-rc2. Guest is centos6. Thanks,
>
> Alex
>
> commit 8b38694a2dc8b18374310df50174f1e4376d6824
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date: Fri Oct 24 14:19:48 2014 +0300
>
> vhost/net: virtio 1.0 byte swap
>
> I had to add an explicit tag to suppress compiler warning:
> gcc isn't smart enough to notice that
> len is always initialized since function is called with size > 0.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
This chunk looks suspicious:
- heads[headcount - 1].len += datalen;
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
s/len - datalen/len + datalen/ ?
> XML chunk:
>
> <interface type='direct'>
> <mac address='52:54:00:64:f3:34'/>
> <source dev='iscsinet0' mode='bridge'/>
> <model type='virtio'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> </interface>
>
> Panic log:
>
> <1>BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> <1>IP: [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> <4>PGD 1aa2f4067 PUD 1aa2f5067 PMD 0
> <4>Oops: 0000 [#1] SMP
> <4>last sysfs file: /sys/devices/pci0000:00/0000:00:03.0/virtio0/net/eth9/ifindex
> <4>CPU 0
> <4>Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last unloaded: speedstep_lib]
> <4>
> <4>Pid: 1374, comm: NetworkManager Tainted: P --------------- 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 1996)
> <4>RIP: 0010:[<ffffffffa0079469>] [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> <4>RSP: 0018:ffff880028203e48 EFLAGS: 00010246
> <4>RAX: ffff8801a3383d00 RBX: ffff8801a6aaf480 RCX: ffff8801aa20b6e0
> <4>RDX: 00000000000000c0 RSI: ffff8801a3383c00 RDI: ffff8801a3383cc0
> <4>RBP: ffff880028203ed8 R08: 000000000000009e R09: ffff8801aa1d800c
> <4>R10: 0000000000000218 R11: 0000000000000000 R12: ffff8801aa20b6e0
> <4>R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> <4>FS: 00007febf114d800(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
> <4>CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>CR2: 0000000000000010 CR3: 00000001aa793000 CR4: 00000000000006f0
> <4>DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> <4>DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> <4>Process NetworkManager (pid: 1374, threadinfo ffff8801a74ba000, task ffff8801a8d56040)
> <4>Stack:
> <4> ffff8801aa1d8000 000000000000009e ffff8801aa20b6e0 ffff8801aa20b718
> <4><d> ffff8801aa20b780 ffff8801aa1d800c ffff8801a6aaf4b8 ffff8801aa20b020
> <4><d> 0000000000000080 ffff8801aa20b708 0000000000000001 00001f5981a830c8
> <4>Call Trace:
> <4> <IRQ>
> <4> [<ffffffff8146ae33>] net_rx_action+0x103/0x2f0
> <4> [<ffffffff8107a5f1>] __do_softirq+0xc1/0x1e0
> <4> [<ffffffff8100c30c>] ? call_softirq+0x1c/0x30
> <4> [<ffffffff8100c30c>] call_softirq+0x1c/0x30
> <4> <EOI>
> <4> [<ffffffff8100fa75>] ? do_softirq+0x65/0xa0
> <4> [<ffffffff8107b2ea>] local_bh_enable+0x9a/0xb0
> <4> [<ffffffffa007813a>] virtnet_napi_enable+0x4a/0x60 [virtio_net]
> <4> [<ffffffffa0078ebf>] virtnet_open+0x4f/0x60 [virtio_net]
> <4> [<ffffffff81467691>] dev_open+0xa1/0x100
> <4> [<ffffffff81466751>] dev_change_flags+0xa1/0x1d0
> <4> [<ffffffff81474a59>] do_setlink+0x169/0x8b0
> <4> [<ffffffff814770b6>] ? rtnl_fill_ifinfo+0x946/0xcb0
> <4> [<ffffffff812a3d24>] ? nla_parse+0x34/0x110
> <4> [<ffffffff8147659e>] rtnl_setlink+0xee/0x130
> <4> [<ffffffff81475b67>] rtnetlink_rcv_msg+0x2d7/0x340
> <4> [<ffffffff81231e14>] ? socket_has_perm+0x74/0x90
> <4> [<ffffffff81475890>] ? rtnetlink_rcv_msg+0x0/0x340
> <4> [<ffffffff814910a9>] netlink_rcv_skb+0xa9/0xd0
> <4> [<ffffffff81475875>] rtnetlink_rcv+0x25/0x40
> <4> [<ffffffff81490cdb>] netlink_unicast+0x2db/0x320
> <4> [<ffffffff81491750>] netlink_sendmsg+0x2c0/0x3d0
> <4> [<ffffffff814520c3>] sock_sendmsg+0x123/0x150
> <4> [<ffffffff81453d73>] ? sock_recvmsg+0x133/0x160
> <4> [<ffffffff8109afa0>] ? autoremove_wake_function+0x0/0x40
> <4> [<ffffffff81136941>] ? lru_cache_add_lru+0x21/0x40
> <4> [<ffffffff8115522d>] ? page_add_new_anon_rmap+0x9d/0xf0
> <4> [<ffffffff8114aeef>] ? handle_pte_fault+0x4af/0xb00
> <4> [<ffffffff81451f14>] ? move_addr_to_kernel+0x64/0x70
> <4> [<ffffffff814538b6>] __sys_sendmsg+0x406/0x420
> <4> [<ffffffff8104a98c>] ? __do_page_fault+0x1ec/0x480
> <4> [<ffffffff814523d9>] ? sys_sendto+0x139/0x190
> <4> [<ffffffff8103ea6c>] ? kvm_clock_read+0x1c/0x20
> <4> [<ffffffff81453ad9>] sys_sendmsg+0x49/0x90
> <4> [<ffffffff8100b072>] system_call_fastpath+0x16/0x1b
> <4>Code: 83 e0 00 00 00 00 10 00 00 48 03 93 d0 00 00 00 66 83 42 04 01 8b 93 cc 00 00 00 48 8b b3 d0 00 00 00 80 4c 16 10 20 44 2b 68 0c <4d> 8b 76 10 75 89 e9 d1 fd ff ff 0f 1f 40 00 a8 02 74 0d 0f b6
> <1>RIP [<ffffffffa0079469>] virtnet_poll+0x4f9/0x910 [virtio_net]
> <4> RSP <ffff880028203e48>
> <4>CR2: 0000000000000010
>
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>
^ permalink raw reply
* My name is Gatan Magsino
From: Sanders M, Tjeu @ 2015-01-07 8:00 UTC (permalink / raw)
My name is Gatan Magsino, I work with Mediterranean Bank in Malta. Can i trust you with a business worth 8.3 million USD? Please reply ONLY to my private email: mgatan@rogers.com<mailto:mgatan@rogers.com> for more information.
^ permalink raw reply
* Re: ipv6: oops in datagram.c line 260
From: Steffen Klassert @ 2015-01-07 7:22 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Chris Ruehl, netdev, davem
In-Reply-To: <1420560073.32369.60.camel@redhat.com>
On Tue, Jan 06, 2015 at 05:01:13PM +0100, Hannes Frederic Sowa wrote:
>
> xfrm6_output_finish unconditionally resets skb->protocol so we try to
> dispatch to the IPv6 handler, even though tcp just sends an IPv4 packet.
Maybe we better dispatch based on sk->sk_family, this should give
always the right address family of the socket.
^ permalink raw reply
* Re: [PATCH] brcm80211: brcmsmac: dma: Remove some unused functions
From: Julia Lawall @ 2015-01-07 6:29 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Arend van Spriel, Kalle Valo, Larry Finger, Brett Rudley,
Hante Meuleman, Fabian Frederick, linux-wireless@vger.kernel.org,
brcm80211-dev-list, Network Development,
Linux Kernel Mailing List
In-Reply-To: <CAKXHbyNBV6=DPF4n0onzhEqAuS-PqLS5pwZj_OitRuYMvgpjvw@mail.gmail.com>
On Wed, 7 Jan 2015, Rickard Strandqvist wrote:
> 2015-01-05 12:06 GMT+01:00 Arend van Spriel <arend@broadcom.com>:
> > On 01/05/15 11:49, Kalle Valo wrote:
> >>
> >> Rickard Strandqvist<rickard_strandqvist@spectrumdigital.se> writes:
> >>
> >>> As I hope you can see I have made some changes regarding the
> >>> subject-line. Thought it was an advantage to be able to see which file
> >>> I actually removed something from. There seems to be a big focus on
> >>> getting right on subject-line right in recent weeks.
> >>>
> >>> I wonder why there is a script that takes a file name, and respond
> >>> with an appropriate subject line?
> >
> >
> > Is there a script for this? Anyway, I would say driver name is enough.
> > Enough about the subject line ;-) I would like to give some general remarks
> > as you seem to touch a lot of kernel code. First off, I think it is good to
> > remove unused stuff. However, I would like some more explanation on your
> > methodology apart from "partially found by using a static code analysis
> > program". So a cover-letter explaining that would have been nice (maybe
> > still is). Things like Kconfig option can affect whether function are used
> > or not so how did you cover that.
> >
> > Regards,
> > Arend
> >
> >
> >> I don't think you can really automate this as some drivers do this a bit
> >> differently. You always need to manually check the commit log.
> >>
> >>> But ok, I change my script accordingly. Should I submit the patch again?
> >>
> >>
> >> Yes, please resubmit.
> >>
> >
>
> Hi Arend
>
> Yes, a script that had been excellent, I think!
> I have one as part of my git send-email script, until a week ago, it
> was enough that I removed the "drivers/" and changed all "/" to ": "
> I have now been expanded my sed pipe a lot (tell me if anyone is interested)
> But now I've seen everything from uppercase and [DIR], etc.
> So I can not understand how anyone should be able to get the right
> name without a good help.
>
> Sure i like to share how I use cppcheck, but is very hesitant to write
> this with each patch mails I send though!
>
> I run:
> cppcheck --force --quiet --enable=all .
>
> Or a specific file instead of .
>
> This will include, among other things get a lot of error message such,
> +4000 for the kernel.
> (style) The function 'xxx' is never used
>
> For these I made a script that searched through all the files after
> the function name (cppcheck missed a few). And save the rest so I go
> through them and possibly send patches.
I think that the question was about what methodology is cppcheck using to
find the given issue. But probably cppcheck is a black box that does
whatever it does, so the user doesn't know what the rationale is.
However, I think you mentioned that cppcheck found only some of the
issues. You could thus describe what was the methodology for finding the
other ones.
julia
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Do not use private netdev_vport fields
From: Pravin Shelar @ 2015-01-07 6:00 UTC (permalink / raw)
To: Daniele Di Proietto; +Cc: David Miller, netdev
In-Reply-To: <CAExiqTLJG38DX_6C2Gaf6jE8-GThznML_-nkzs+57fU72QPW7w@mail.gmail.com>
On Tue, Jan 6, 2015 at 3:32 PM, Daniele Di Proietto
<daniele.di.proietto@gmail.com> wrote:
> The motivation for the change is to make datapath.c independent from
> the actual implementation of the vport. The problem came up when
> experimenting with other vport implementations and this type of change
> will help identifying layering violations.
> You are perfectly right, however, that in this form the code does not
> improve much: we should rather provide a vport_index() call, and
> implement one in each of the vports.
>
This sounds like lot more invasive change compared to the current
patch. For such change I need to see complete set of changes that you
are planning.
> Thoughts?
>
> 2015-01-06 23:28 GMT+01:00 Pravin Shelar <pshelar@nicira.com>:
>> On Tue, Jan 6, 2015 at 2:15 PM, Pravin Shelar <pshelar@nicira.com> wrote:
>>> On Tue, Jan 6, 2015 at 2:02 PM, David Miller <davem@davemloft.net> wrote:
>>>> From: Pravin Shelar <pshelar@nicira.com>
>>>> Date: Tue, 6 Jan 2015 13:16:11 -0800
>>>>
>>>>> Function return type and function name should be on same line,
>>>>> otherwise looks good.
>>>>
>>>> I disagree, where is the code in the tree that needs this?
>>>
>>> Most of function definitions that I have seen are defined like this. I
>>> was pointing out coding style issue.
>>
>> About the actual change, I think it is a cleanup. netdev_vport_index()
>> hides the implementation from datapath.c. I hope Daniele will explain
>> need for the change.
^ permalink raw reply
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Fan Du @ 2015-01-07 5:58 UTC (permalink / raw)
To: Jesse Gross
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Michael S. Tsirkin,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Wang,
Du, Fan, fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <CAEP_g=8bCR=PeSoi09jLWLtNUrxhzx45h1Wm=9D=R57AqUac2w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
于 2015年01月07日 03:11, Jesse Gross 写道:
>>> One of the reasons for only doing path MTU discovery
>>> >>for L3 is that it operates seamlessly as part of normal operation -
>>> >>there is no need to forge addresses or potentially generate ICMP when
>>> >>on an L2 network. However, this ignores the IP handling that is going
>>> >>on (note that in OVS it is possible for L3 to be implemented as a set
>>> >>of flows coming from a controller).
>>> >>
>>> >>It also should not be VXLAN specific or duplicate VXLAN encapsulation
>>> >>code. As this is happening before encapsulation, the generated ICMP
>>> >>does not need to be encapsulated either if it is created in the right
>>> >>location.
>> >
>> >Yes, I agree. GRE share the same issue from the code flow.
>> >Pushing back ICMP msg back without encapsulation without circulating down
>> >to physical device is possible. The "right location" as far as I know
>> >could only be in ovs_vport_send. In addition this probably requires wrapper
>> >route looking up operation for GRE/VXLAN, after get the under layer device
>> >MTU
>> >from the routing information, then calculate reduced MTU becomes feasible.
> As I said, it needs to be integrated into L3 processing. In OVS this
> would mean adding some primitives to the kernel and then exposing the
> functionality upwards into userspace/controller.
I'm bit of confused with "L3 processing" you mentioned here... SORRY
Apparently I'm not seeing the whole picture as you pointed out. Could you please
elaborate "L3 processing" a bit more? docs/codes/or other useful links. Appreciated.
My understanding is:
controller sets the forwarding rules into kernel datapath, any flow not matching
with the rules are threw to controller by upcall. Once the rule decision is made
by controller, then, this flow packet is pushed down to datapath to be forwarded
again according to the new rule.
So I'm not sure whether pushing the over-MTU-sized packet or pushing the forged ICMP
without encapsulation to controller is required by current ovs implementation. By doing
so, such over-MTU-sized packet is treated as a event for the controller to be take
care of.
--
No zuo no die but I have to try.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [net v2 2/3] i40e: Fix Rx checksum error counter
From: Tom Herbert @ 2015-01-07 5:43 UTC (permalink / raw)
To: Jeff Kirsher
Cc: David Miller, Anjali Singhai, Linux Netdev List, nhorman,
sassmann, jogreene, Greg Rose
In-Reply-To: <1420594317-6191-3-git-send-email-jeffrey.t.kirsher@intel.com>
On Tue, Jan 6, 2015 at 5:31 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Anjali Singhai <anjali.singhai@intel.com>
>
> The Rx port checksum error counter was incrementing incorrectly with
> UDP encapsulated tunneled traffic. This patch fixes the problem so that
> the port_rx_csum counter will show accurate statistics.
>
> Signed-off-by: Anjali Singhai <anjali.singhai@intel.com>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Tested-by: Jim Young <james.m.young@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index f145aaf..38c7638 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -1325,9 +1325,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
> * so the total length of IPv4 header is IHL*4 bytes
> * The UDP_0 bit *may* bet set if the *inner* header is UDP
> */
> - if (ipv4_tunnel &&
> - (decoded.inner_prot != I40E_RX_PTYPE_INNER_PROT_UDP) &&
> - !(rx_status & (1 << I40E_RX_DESC_STATUS_UDP_0_SHIFT))) {
> + if (ipv4_tunnel) {
> skb->transport_header = skb->mac_header +
> sizeof(struct ethhdr) +
> (ip_hdr(skb)->ihl * 4);
> @@ -1337,15 +1335,19 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
> skb->protocol == htons(ETH_P_8021AD))
> ? VLAN_HLEN : 0;
>
> - rx_udp_csum = udp_csum(skb);
> - iph = ip_hdr(skb);
> - csum = csum_tcpudp_magic(
> - iph->saddr, iph->daddr,
> - (skb->len - skb_transport_offset(skb)),
> - IPPROTO_UDP, rx_udp_csum);
> + if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
> + (udp_hdr(skb)->check != 0)) {
> + rx_udp_csum = udp_csum(skb);
Doesn't this compute the whole checksum of the packet making the fact
that device verified inner checksum pretty much irrelevant? It would
probably be just as well to return CHECKSUM_NONE and let the stack
deal with it and remove all this complexity.
> + iph = ip_hdr(skb);
> + csum = csum_tcpudp_magic(
> + iph->saddr, iph->daddr,
> + (skb->len - skb_transport_offset(skb)),
> + IPPROTO_UDP, rx_udp_csum);
>
> - if (udp_hdr(skb)->check != csum)
> - goto checksum_fail;
> + if (udp_hdr(skb)->check != csum)
> + goto checksum_fail;
> +
> + } /* else its GRE and so no outer UDP header */
> }
>
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> --
> 1.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
* [PATCH net-next v2 3/7] rhashtable: involve rhashtable_lookup_insert routine
From: Ying Xue @ 2015-01-07 5:41 UTC (permalink / raw)
To: tgraf; +Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
tipc-discussion
In-Reply-To: <1420609318-3261-1-git-send-email-ying.xue@windriver.com>
Involve a new function called rhashtable_lookup_insert() which makes
lookup and insertion atomic under bucket lock protection, helping us
avoid to introduce an extra lock when we search and insert an object
into hash table.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Acked-by: Thomas Graf <tgraf@suug.ch>
---
include/linux/rhashtable.h | 1 +
lib/rhashtable.c | 97 +++++++++++++++++++++++++++++++++++++-------
2 files changed, 83 insertions(+), 15 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index de1459c7..73c913f 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -168,6 +168,7 @@ int rhashtable_shrink(struct rhashtable *ht);
void *rhashtable_lookup(struct rhashtable *ht, const void *key);
void *rhashtable_lookup_compare(struct rhashtable *ht, const void *key,
bool (*compare)(void *, void *), void *arg);
+bool rhashtable_lookup_insert(struct rhashtable *ht, struct rhash_head *obj);
void rhashtable_destroy(struct rhashtable *ht);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 2000685..4430233 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -505,8 +505,26 @@ static void rhashtable_wakeup_worker(struct rhashtable *ht)
schedule_delayed_work(&ht->run_work, 0);
}
+static void __rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj,
+ struct bucket_table *tbl, u32 hash)
+{
+ struct rhash_head *head = rht_dereference_bucket(tbl->buckets[hash],
+ tbl, hash);
+
+ if (rht_is_a_nulls(head))
+ INIT_RHT_NULLS_HEAD(obj->next, ht, hash);
+ else
+ RCU_INIT_POINTER(obj->next, head);
+
+ rcu_assign_pointer(tbl->buckets[hash], obj);
+
+ atomic_inc(&ht->nelems);
+
+ rhashtable_wakeup_worker(ht);
+}
+
/**
- * rhashtable_insert - insert object into hash hash table
+ * rhashtable_insert - insert object into hash table
* @ht: hash table
* @obj: pointer to hash head inside object
*
@@ -523,7 +541,6 @@ static void rhashtable_wakeup_worker(struct rhashtable *ht)
void rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj)
{
struct bucket_table *tbl;
- struct rhash_head *head;
spinlock_t *lock;
unsigned hash;
@@ -534,19 +551,9 @@ void rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj)
lock = bucket_lock(tbl, hash);
spin_lock_bh(lock);
- head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
- if (rht_is_a_nulls(head))
- INIT_RHT_NULLS_HEAD(obj->next, ht, hash);
- else
- RCU_INIT_POINTER(obj->next, head);
-
- rcu_assign_pointer(tbl->buckets[hash], obj);
+ __rhashtable_insert(ht, obj, tbl, hash);
spin_unlock_bh(lock);
- atomic_inc(&ht->nelems);
-
- rhashtable_wakeup_worker(ht);
-
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(rhashtable_insert);
@@ -560,7 +567,7 @@ EXPORT_SYMBOL_GPL(rhashtable_insert);
* walk the bucket chain upon removal. The removal operation is thus
* considerable slow if the hash table is not correctly sized.
*
- * Will automatically shrink the table via rhashtable_expand() if the the
+ * Will automatically shrink the table via rhashtable_expand() if the
* shrink_decision function specified at rhashtable_init() returns true.
*
* The caller must ensure that no concurrent table mutations occur. It is
@@ -641,7 +648,7 @@ static bool rhashtable_compare(void *ptr, void *arg)
* for a entry with an identical key. The first matching entry is returned.
*
* This lookup function may only be used for fixed key hash table (key_len
- * paramter set). It will BUG() if used inappropriately.
+ * parameter set). It will BUG() if used inappropriately.
*
* Lookups may occur in parallel with hashtable mutations and resizing.
*/
@@ -702,6 +709,66 @@ restart:
}
EXPORT_SYMBOL_GPL(rhashtable_lookup_compare);
+/**
+ * rhashtable_lookup_insert - lookup and insert object into hash table
+ * @ht: hash table
+ * @obj: pointer to hash head inside object
+ *
+ * Locks down the bucket chain in both the old and new table if a resize
+ * is in progress to ensure that writers can't remove from the old table
+ * and can't insert to the new table during the atomic operation of search
+ * and insertion. Searches for duplicates in both the old and new table if
+ * a resize is in progress.
+ *
+ * This lookup function may only be used for fixed key hash table (key_len
+ * parameter set). It will BUG() if used inappropriately.
+ *
+ * It is safe to call this function from atomic context.
+ *
+ * Will trigger an automatic deferred table resizing if the size grows
+ * beyond the watermark indicated by grow_decision() which can be passed
+ * to rhashtable_init().
+ */
+bool rhashtable_lookup_insert(struct rhashtable *ht, struct rhash_head *obj)
+{
+ struct bucket_table *new_tbl, *old_tbl;
+ spinlock_t *new_bucket_lock, *old_bucket_lock;
+ u32 new_hash, old_hash;
+ bool success = true;
+
+ BUG_ON(!ht->p.key_len);
+
+ rcu_read_lock();
+
+ old_tbl = rht_dereference_rcu(ht->tbl, ht);
+ old_hash = head_hashfn(ht, old_tbl, obj);
+ old_bucket_lock = bucket_lock(old_tbl, old_hash);
+ spin_lock_bh(old_bucket_lock);
+
+ new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
+ new_hash = head_hashfn(ht, new_tbl, obj);
+ new_bucket_lock = bucket_lock(new_tbl, new_hash);
+ if (unlikely(old_tbl != new_tbl))
+ spin_lock_bh_nested(new_bucket_lock, RHT_LOCK_NESTED);
+
+ if (rhashtable_lookup(ht, rht_obj(ht, obj) + ht->p.key_offset)) {
+ success = false;
+ goto exit;
+ }
+
+ __rhashtable_insert(ht, obj, new_tbl, new_hash);
+
+exit:
+ if (unlikely(old_tbl != new_tbl))
+ spin_unlock_bh(new_bucket_lock);
+ spin_unlock_bh(old_bucket_lock);
+
+ rcu_read_unlock();
+
+ return success;
+}
+EXPORT_SYMBOL_GPL(rhashtable_lookup_insert);
+
static size_t rounded_hashtable_size(struct rhashtable_params *params)
{
return max(roundup_pow_of_two(params->nelem_hint * 4 / 3),
--
1.7.9.5
^ permalink raw reply related
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