* Re: [PATCH] net: add QCA alx Ethernet driver
From: David Miller @ 2012-03-30 20:45 UTC (permalink / raw)
To: rodrigue
Cc: xiong, netdev, linux-kernel, qca-linux-team, nic-devel, kgiori,
chris.snook, mathieu, bryanh, jespera, julia
In-Reply-To: <CAB=NE6WPWmrTZ__JQViqhde-krBsbM5G+ZQgK0pNC+W909u_ew@mail.gmail.com>
From: "Luis R. Rodriguez" <rodrigue@qca.qualcomm.com>
Date: Fri, 30 Mar 2012 10:10:55 -0700
> On Wed, Mar 21, 2012 at 6:28 PM, Luis R. Rodriguez
> <rodrigue@qca.qualcomm.com> wrote:
>> On Tue, Feb 28, 2012 at 7:32 PM, David Miller <davem@davemloft.net> wrote:
>> Would it be worthwhile to consider alx upstream only for the newer
>> chipsets (regardless of the litmus test, which I do agree with on
>> technical grounds) in consideration for helping pave the way on
>> killing proprietary drivers?
...
> David, please us know if you think the above reason is worthy for
> consideration of alx upstream, if at least for the newer chipsets.
Submitting alx that only supports the newer chipsets is fine.
^ permalink raw reply
* Re: [PATCH] net: add QCA alx Ethernet driver
From: Luis R. Rodriguez @ 2012-03-30 20:52 UTC (permalink / raw)
To: David Miller
Cc: xiong, netdev, linux-kernel, qca-linux-team, nic-devel, kgiori,
chris.snook, mathieu, bryanh, jespera, julia
In-Reply-To: <20120330.164554.2217665316338460995.davem@davemloft.net>
On Fri, Mar 30, 2012 at 04:45:54PM -0400, David Miller wrote:
> From: "Luis R. Rodriguez" <rodrigue@qca.qualcomm.com>
> Date: Fri, 30 Mar 2012 10:10:55 -0700
>
> > On Wed, Mar 21, 2012 at 6:28 PM, Luis R. Rodriguez
> > <rodrigue@qca.qualcomm.com> wrote:
> >> On Tue, Feb 28, 2012 at 7:32 PM, David Miller <davem@davemloft.net> wrote:
> >> Would it be worthwhile to consider alx upstream only for the newer
> >> chipsets (regardless of the litmus test, which I do agree with on
> >> technical grounds) in consideration for helping pave the way on
> >> killing proprietary drivers?
> ...
> > David, please us know if you think the above reason is worthy for
> > consideration of alx upstream, if at least for the newer chipsets.
>
> Submitting alx that only supports the newer chipsets is fine.
We will work on this and resubmit, the delta for the other chipsets
can also be addressed within atl1c, in addition to working on getting
documentation out to help enable further.
Luis
^ permalink raw reply
* adding ipv6 address manually in Linux
From: Adishesh M @ 2012-03-30 20:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <CA+f1OFri1pvyn2n=L82bip+KEY4QnXEwPxNvy=23UOR4vDTzJQ@mail.gmail.com>
Hi,
there some problem in linux if we send 'unsolicited Neighbor
Advertisement' message for an ipv6 address as explained below.
Manually add a new ipv6 address in to a Linux machine and send
immediately 'unsolicited Neighbor Advertisement' then newly added
address is considered as duplicate ipv6 address.
usually if a new ipv6 address is added then Linux is sending '
Neighbor solicitation message in order to detect duplicate address. By some
how if i send unsolicited Neighbor advertisement message before linux
sends 'neighbor solicitation' message then linux is marking this
address as duplicate.
'ip -6 addr show' command output is
inet6 2001:3333:5678:ddd1::55/64 scope global tentative dadfailed
why Linux is considering ipv6 address as duplicate if we send
unsolicited message?
if unsolicited message is sent after Linux completes duplicate address
detection procedure by using neighbor solicitation message then this
address is not considered as duplicate.
Regards,
Adishesh
^ permalink raw reply
* [PATCH] tilepro ethernet driver: fix a few minor issues
From: Chris Metcalf @ 2012-03-30 23:23 UTC (permalink / raw)
To: Chris Metcalf, netdev, linux-kernel
This commit fixes a number of issues seen with the driver:
- Improve handling of return credits to the hardware shim
- Use skb_frag_size() appropriately
- Add netpoll support to run console over UDP
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
drivers/net/ethernet/tile/tilepro.c | 75 +++++++++++++++++++++++------------
1 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/tile/tilepro.c b/drivers/net/ethernet/tile/tilepro.c
index d9951af..eb4ea08 100644
--- a/drivers/net/ethernet/tile/tilepro.c
+++ b/drivers/net/ethernet/tile/tilepro.c
@@ -342,6 +342,21 @@ inline int __netio_fastio1(u32 fastio_index, u32 arg0)
}
+static void tile_net_return_credit(struct tile_net_cpu *info)
+{
+ struct tile_netio_queue *queue = &info->queue;
+ netio_queue_user_impl_t *qup = &queue->__user_part;
+
+ /* Return four credits after every fourth packet. */
+ if (--qup->__receive_credit_remaining == 0) {
+ u32 interval = qup->__receive_credit_interval;
+ qup->__receive_credit_remaining = interval;
+ __netio_fastio_return_credits(qup->__fastio_index, interval);
+ }
+}
+
+
+
/*
* Provide a linux buffer to LIPP.
*/
@@ -864,19 +879,11 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
stats->rx_packets++;
stats->rx_bytes += len;
-
- if (small)
- info->num_needed_small_buffers++;
- else
- info->num_needed_large_buffers++;
}
- /* Return four credits after every fourth packet. */
- if (--qup->__receive_credit_remaining == 0) {
- u32 interval = qup->__receive_credit_interval;
- qup->__receive_credit_remaining = interval;
- __netio_fastio_return_credits(qup->__fastio_index, interval);
- }
+ /* ISSUE: It would be nice to defer this until the packet has */
+ /* actually been processed. */
+ tile_net_return_credit(info);
/* Consume this packet. */
qup->__packet_receive_read = index2;
@@ -1543,7 +1550,7 @@ static int tile_net_drain_lipp_buffers(struct tile_net_priv *priv)
/* Drain all the LIPP buffers. */
while (true) {
- int buffer;
+ unsigned int buffer;
/* NOTE: This should never fail. */
if (hv_dev_pread(priv->hv_devhdl, 0, (HV_VirtAddr)&buffer,
@@ -1707,7 +1714,7 @@ static unsigned int tile_net_tx_frags(lepp_frag_t *frags,
if (!hash_default) {
void *va = pfn_to_kaddr(pfn) + f->page_offset;
BUG_ON(PageHighMem(skb_frag_page(f)));
- finv_buffer_remote(va, f->size, 0);
+ finv_buffer_remote(va, skb_frag_size(f), 0);
}
cpa = ((phys_addr_t)pfn << PAGE_SHIFT) + f->page_offset;
@@ -1735,8 +1742,8 @@ static unsigned int tile_net_tx_frags(lepp_frag_t *frags,
* Sometimes, if "sendfile()" requires copying, we will be called with
* "data" containing the header and payload, with "frags" being empty.
*
- * In theory, "sh->nr_frags" could be 3, but in practice, it seems
- * that this will never actually happen.
+ * Sometimes, for example when using NFS over TCP, a single segment can
+ * span 3 fragments, which must be handled carefully in LEPP.
*
* See "emulate_large_send_offload()" for some reference code, which
* does not handle checksumming.
@@ -1844,10 +1851,8 @@ static int tile_net_tx_tso(struct sk_buff *skb, struct net_device *dev)
spin_lock_irqsave(&priv->eq_lock, irqflags);
- /*
- * Handle completions if needed to make room.
- * HACK: Spin until there is sufficient room.
- */
+ /* Handle completions if needed to make room. */
+ /* NOTE: Return NETDEV_TX_BUSY if there is still no room. */
if (lepp_num_free_comp_slots(eq) == 0) {
nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
if (nolds == 0) {
@@ -1861,6 +1866,7 @@ busy:
cmd_tail = eq->cmd_tail;
/* Prepare to advance, detecting full queue. */
+ /* NOTE: Return NETDEV_TX_BUSY if the queue is full. */
cmd_next = cmd_tail + cmd_size;
if (cmd_tail < cmd_head && cmd_next >= cmd_head)
goto busy;
@@ -2023,10 +2029,8 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
spin_lock_irqsave(&priv->eq_lock, irqflags);
- /*
- * Handle completions if needed to make room.
- * HACK: Spin until there is sufficient room.
- */
+ /* Handle completions if needed to make room. */
+ /* NOTE: Return NETDEV_TX_BUSY if there is still no room. */
if (lepp_num_free_comp_slots(eq) == 0) {
nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
if (nolds == 0) {
@@ -2040,6 +2044,7 @@ busy:
cmd_tail = eq->cmd_tail;
/* Copy the commands, or fail. */
+ /* NOTE: Return NETDEV_TX_BUSY if the queue is full. */
for (i = 0; i < num_frags; i++) {
/* Prepare to advance, detecting full queue. */
@@ -2260,6 +2265,23 @@ static int tile_net_get_mac(struct net_device *dev)
return 0;
}
+
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/*
+ * Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void tile_net_netpoll(struct net_device *dev)
+{
+ struct tile_net_priv *priv = netdev_priv(dev);
+ disable_percpu_irq(priv->intr_id);
+ tile_net_handle_ingress_interrupt(priv->intr_id, dev);
+ enable_percpu_irq(priv->intr_id, 0);
+}
+#endif
+
+
static const struct net_device_ops tile_net_ops = {
.ndo_open = tile_net_open,
.ndo_stop = tile_net_stop,
@@ -2268,7 +2290,10 @@ static const struct net_device_ops tile_net_ops = {
.ndo_get_stats = tile_net_get_stats,
.ndo_change_mtu = tile_net_change_mtu,
.ndo_tx_timeout = tile_net_tx_timeout,
- .ndo_set_mac_address = tile_net_set_mac_address
+ .ndo_set_mac_address = tile_net_set_mac_address,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = tile_net_netpoll,
+#endif
};
@@ -2408,7 +2433,7 @@ static void tile_net_cleanup(void)
*/
static int tile_net_init_module(void)
{
- pr_info("Tilera IPP Net Driver\n");
+ pr_info("Tilera Network Driver\n");
tile_net_devs[0] = tile_net_dev_init("xgbe0");
tile_net_devs[1] = tile_net_dev_init("xgbe1");
--
1.6.5.2
^ permalink raw reply related
* Re: sendmmsg: put_user vs __put_user
From: David Miller @ 2012-03-31 0:51 UTC (permalink / raw)
To: drepper; +Cc: netdev, linux-kernel
In-Reply-To: <CAOPLpQf2bmj+i_zYtsS8JNzTDuBxCO5ByNRRqwv0kxaDBFdezg@mail.gmail.com>
From: Ulrich Drepper <drepper@gmail.com>
Date: Fri, 30 Mar 2012 09:36:11 -0400
> Shouldn't the compat code in the sendmmsg implementation use the same
> code as the normal code? In which case you probably want something
> like this:
Compat processes are not able to generate virtual addresses anywhere
near the range where the kernel resides, so the address range
verification done by put_user() is completely superfluous and
therefore not necessary. The normal exception handling done by the
access is completely sufficient.
^ permalink raw reply
* Re: [PATCH] Fix NULL pointer dereference on firmware name for early calls to get_drvinfo.
From: David Miller @ 2012-03-31 0:51 UTC (permalink / raw)
To: notting; +Cc: netdev, wimax, inaky.perez-gonzalez
In-Reply-To: <1333139045-18170-1-git-send-email-notting@redhat.com>
From: Bill Nottingham <notting@redhat.com>
Date: Fri, 30 Mar 2012 16:24:05 -0400
> The driver comments show an initialization sequence of:
> * i2400m_setup()
> * i2400m->bus_setup()
> * i2400m_bootrom_init()
> * register_netdev()
> * wimax_dev_add()
> * i2400m_dev_start()
> * __i2400m_dev_start()
> * i2400m_dev_bootstrap()
>
> dev_bootstrap() is where the firmware is loaded. So, if something calls
> get_drvinfo() from a register_netdevice_notifier (such as the cnic driver),
> we won't have a firmware name, and strncpy will crash.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=808603
>
> Signed-off-by: Bill Nottingham <notting@redhat.com>
Already fixed in current sources.
^ permalink raw reply
* 答复: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
From: Peter Huang (Peng) @ 2012-03-31 1:26 UTC (permalink / raw)
To: 'Eric Dumazet'
Cc: linux-kernel, harry.majun, zhoukang7, 'netdev'
In-Reply-To: <1333020682.2325.517.camel@edumazet-glaptop>
> > >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> > >DST_NOPEER dst flag ) 4adf0af6818f3ea5 (bridge: send correct MTU value
> > >in PMTU (revised))
Hi, Eric
I confirmed the patch contents again.
For our case, NF_INET_PRE_ROUTING is involved, not NF_BR_LOCAL_IN.
It seems pre-routing is not included in the patch you mentioned.
BTW, our pkts are all ipv6 DHCP pkts(with MAC 33 33 00 01 00 02).
We verified the patch bellow, and this works fine, no WARN_ON happened.
--------------------------------
Peter Huang(peng)
> On Thu, 2012-03-29 at 17:38 +0800, Peter Huang (Peng) wrote:
> > Thks for your mail.
> >
> > >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> > >DST_NOPEER dst flag ) 4adf0af6818f3ea5 (bridge: send correct MTU value
> > >in PMTU (revised))
> >
> > This patch already included in kernel-3.3, but for our case, virtual tap device's delayed
> > Deletion will also cause kernel oops even in kernel3.3.
>
> I was suggesting you take a look at the commit content ;)
>
> Then you can see the code in br_nf_local_in(), a bit cleaner than yours.
>
^ permalink raw reply
* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: Vladimir Murzin @ 2012-03-31 4:40 UTC (permalink / raw)
To: Will Drewry
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, akpm, corbet, eric.dumazet, markus,
coreyb, keescook, jmorris
In-Reply-To: <1333051320-30872-9-git-send-email-wad@chromium.org>
Hi Will,
On Thu, Mar 29, 2012 at 03:01:53PM -0500, Will Drewry wrote:
snipped
> +
> +/* Limit any path through the tree to 256KB worth of instructions. */
> +#define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))
> +
> +static void seccomp_filter_log_failure(int syscall)
> +{
> + int compat = 0;
> +#ifdef CONFIG_COMPAT
> + compat = is_compat_task();
> +#endif
> + pr_info("%s[%d]: %ssystem call %d blocked at 0x%lx\n",
> + current->comm, task_pid_nr(current),
> + (compat ? "compat " : ""),
> + syscall, KSTK_EIP(current));
> +}
snipped
> +/**
> + * seccomp_attach_user_filter - attaches a user-supplied sock_fprog
> + * @user_filter: pointer to the user data containing a sock_fprog.
> + *
> + * Returns 0 on success and non-zero otherwise.
> + */
> +long seccomp_attach_user_filter(char __user *user_filter)
> +{
> + struct sock_fprog fprog;
> + long ret = -EFAULT;
> +
> +#ifdef CONFIG_COMPAT
> + if (is_compat_task()) {
> + struct compat_sock_fprog fprog32;
> + if (copy_from_user(&fprog32, user_filter, sizeof(fprog32)))
> + goto out;
> + fprog.len = fprog32.len;
> + fprog.filter = compat_ptr(fprog32.filter);
> + } else /* falls through to the if below. */
> +#endif
> + if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
> + goto out;
> + ret = seccomp_attach_filter(&fprog);
> +out:
> + return ret;
> +}
Do we really need to surround is_compat_task() with CNFIG_COMPAT?
It seems that this case has already handled in compat.h [1]
[1] http://lxr.linux.no/#linux+v3.3/include/linux/compat.h#L566
Best wishes
Vladimir Murzin
^ permalink raw reply
* Re: 答复: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
From: Eric Dumazet @ 2012-03-31 5:41 UTC (permalink / raw)
To: Peter Huang (Peng); +Cc: linux-kernel, harry.majun, zhoukang7, 'netdev'
In-Reply-To: <004d01cd0edd$593f3bd0$0bbdb370$%huangpeng@huawei.com>
On Sat, 2012-03-31 at 09:26 +0800, Peter Huang (Peng) wrote:
> > > >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> > > >DST_NOPEER dst flag ) 4adf0af6818f3ea5 (bridge: send correct MTU value
> > > >in PMTU (revised))
>
> Hi, Eric
>
> I confirmed the patch contents again.
> For our case, NF_INET_PRE_ROUTING is involved, not NF_BR_LOCAL_IN.
> It seems pre-routing is not included in the patch you mentioned.
> BTW, our pkts are all ipv6 DHCP pkts(with MAC 33 33 00 01 00 02).
>
> We verified the patch bellow, and this works fine, no WARN_ON happened.
Hi Peter
I claim that your patch is not the good one and you need to refine it.
First, code is not needed if CONFIG_BRIDGE_NETFILTER is not set.
In fact, if CONFIG_BRIDGE_NETFILTER is not set, compilation will fail
since fake_rtable doesnt exist in "struct net_bridge", so you fix a bug
and introduce a new one.
CC [M] net/bridge/br_forward.o
net/bridge/br_forward.c: In function ‘__br_forward’:
net/bridge/br_forward.c:94: error: ‘struct sk_buff’ has no member named ‘_skb_dst’
net/bridge/br_forward.c:94: error: ‘struct net_bridge’ has no member named ‘fake_rtable’
make[1]: *** [net/bridge/br_forward.o] Error 1
make: *** [_module_net/bridge] Error 2
Then, the test is using obsolete dst internals and cast that should not
be in a C file. :
if (skb->_skb_dst == (unsigned long)&to->br->fake_rtable)
So I suggested you take a look at net/bridge/br_netfilter.c code to see
how this can be done properly. Maybe you need to add a helper in an
include file to make this proper.
I hope this is now clear to you, because your initial patch cannot be
applied as is. If you want full credit for this work, you must go a step
forward, or else another guy will finish the job.
Thanks
^ permalink raw reply
* 答复: 答复: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
From: Peter Huang (Peng) @ 2012-03-31 7:29 UTC (permalink / raw)
To: 'Eric Dumazet'
Cc: linux-kernel, harry.majun, zhoukang7, 'netdev'
In-Reply-To: <1333172486.2325.3101.camel@edumazet-glaptop>
Hi, Eric
Thanks very much for your reply.
I will refine this patch and send it out later.
--------------------------------
Peter Huang(peng)
> -----邮件原件-----
> 发件人: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> 发送时间: 2012年3月31日 13:41
> 收件人: Peter Huang (Peng)
> 抄送: linux-kernel@vger.kernel.org; harry.majun@huawei.com; zhoukang7@huawei.com; 'netdev'
> 主题: Re: 答复: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
>
> On Sat, 2012-03-31 at 09:26 +0800, Peter Huang (Peng) wrote:
> > > > >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> > > > >DST_NOPEER dst flag ) 4adf0af6818f3ea5 (bridge: send correct MTU value
> > > > >in PMTU (revised))
> >
> > Hi, Eric
> >
> > I confirmed the patch contents again.
> > For our case, NF_INET_PRE_ROUTING is involved, not NF_BR_LOCAL_IN.
> > It seems pre-routing is not included in the patch you mentioned.
> > BTW, our pkts are all ipv6 DHCP pkts(with MAC 33 33 00 01 00 02).
> >
> > We verified the patch bellow, and this works fine, no WARN_ON happened.
>
> Hi Peter
>
> I claim that your patch is not the good one and you need to refine it.
>
> First, code is not needed if CONFIG_BRIDGE_NETFILTER is not set.
>
> In fact, if CONFIG_BRIDGE_NETFILTER is not set, compilation will fail
> since fake_rtable doesnt exist in "struct net_bridge", so you fix a bug
> and introduce a new one.
>
> CC [M] net/bridge/br_forward.o
> net/bridge/br_forward.c: In function ‘__br_forward’:
> net/bridge/br_forward.c:94: error: ‘struct sk_buff’ has no member named ‘_skb_dst’
> net/bridge/br_forward.c:94: error: ‘struct net_bridge’ has no member named ‘fake_rtable’
> make[1]: *** [net/bridge/br_forward.o] Error 1
> make: *** [_module_net/bridge] Error 2
>
>
> Then, the test is using obsolete dst internals and cast that should not
> be in a C file. :
>
> if (skb->_skb_dst == (unsigned long)&to->br->fake_rtable)
>
> So I suggested you take a look at net/bridge/br_netfilter.c code to see
> how this can be done properly. Maybe you need to add a helper in an
> include file to make this proper.
>
> I hope this is now clear to you, because your initial patch cannot be
> applied as is. If you want full credit for this work, you must go a step
> forward, or else another guy will finish the job.
>
> Thanks
^ permalink raw reply
* Re: question about frag_can_reassemble()
From: Marek Lindner @ 2012-03-31 8:52 UTC (permalink / raw)
To: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Dan Carpenter
In-Reply-To: <20120330200236.GH3289@mwanda>
Dan,
> I had a question about the code in frag_can_reassemble().
>
> net/batman-adv/unicast.h
> 51
> 52 merged_size = (skb->len - sizeof(*unicast_packet)) * 2;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 53 merged_size += sizeof(struct unicast_packet) +
> uneven_correction; 54
> 55 return merged_size <= mtu;
> 56 }
>
> Can the skb->len be less than sizeof(*unicast_packet) (ie 20 bytes)?
> If "len" is less than 10 then we would return false but if it's
> over 10 then we would return true. Roughly.
the calling function checks for an existing unicast header. The call tree
looks like this:
recv_ucast_frag_packet() -> route_unicast_packet() -> frag_can_reassemble()
The first function does a unicast length check by calling
check_unicast_packet(). As a result we ensure that we at least have the length
of the unicast header in our packet unless we overlooked something ?
Regards,
Marek
^ permalink raw reply
* Re: [PATCH v5 2/2] Ethernet driver for the WIZnet W5100 chip
From: Francois Romieu @ 2012-03-31 9:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Mike Sinkovsky, netdev, linux-kernel
In-Reply-To: <1333099431.19659.15.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> :
> Le vendredi 30 mars 2012 ?? 13:00 +0600, Mike Sinkovsky a ??crit :
> > Tested and used in production with Blackfin BF531 embedded processor.
[...]
> > +static int w5100_start_tx(struct sk_buff *skb, struct net_device *ndev)
> > +{
> > + struct w5100_priv *priv = netdev_priv(ndev);
> > + u16 offset;
> > +
> > + if (w5100_read16(priv, W5100_S0_TX_FSR) < skb->len) {
>
> There is a race here.
>
> Interrupt came come right here before you set the stop_queue()
>
>
> > + netif_stop_queue(ndev);
>
> So Here I suggest adding a test again
>
> if (w5100_read16(priv, W5100_S0_TX_FSR) >= skb->len)
> netif_wake_queue(ndev);
> else
> return NETDEV_TX_BUSY;
The start_tx logic seems broken anyway: it claims it always can take
packets until it is given one it can't process. The driver should rather
do the opposite and stop queueing as soon as there is no guarantee for
the next packet.
--
Ueimor
^ permalink raw reply
* Re: [PATCH v5 2/2] Ethernet driver for the WIZnet W5100 chip
From: Francois Romieu @ 2012-03-31 9:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Mike Sinkovsky, netdev, linux-kernel
In-Reply-To: <20120331094014.GA7746@electric-eye.fr.zoreil.com>
Francois Romieu <romieu@fr.zoreil.com> :
[...]
> The start_tx logic seems broken anyway: it claims it always can take
> packets until it is given one it can't process. The driver should rather
> do the opposite and stop queueing as soon as there is no guarantee for
> the next packet.
Forget it: the driver stops queuing in this path anyway.
--
Ueimor
^ permalink raw reply
* [PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
From: Indan Zupancic @ 2012-03-31 9:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, stable, linux-kernel
Hello,
Finally, after much searching I found one little bug.
[PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
Small typo resulted in bad code generation for certain
values of K for the BPF_S_ALU_AND_K instruction.
Signed-off-by: Indan Zupancic <indan@nul.nu>
---
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 7c1b765..28bc807 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -289,7 +289,7 @@ void bpf_jit_compile(struct sk_filter *fp)
EMIT2(0x24, K & 0xFF); /* and imm8,%al */
} else if (K >= 0xFFFF0000) {
EMIT2(0x66, 0x25); /* and imm16,%ax */
- EMIT2(K, 2);
+ EMIT(K, 2);
} else {
EMIT1_off32(0x25, K); /* and imm32,%eax */
}
^ permalink raw reply related
* Re: [PATCH v5 2/2] Ethernet driver for the WIZnet W5100 chip
From: Mike Sinkovsky @ 2012-03-31 9:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, linux-kernel
In-Reply-To: <1333099431.19659.15.camel@edumazet-laptop>
30.03.2012 15:23, Eric Dumazet написал:
>> +static int w5100_start_tx(struct sk_buff *skb, struct net_device *ndev)
>> +{
>> + struct w5100_priv *priv = netdev_priv(ndev);
>> + u16 offset;
>> +
>> + if (w5100_read16(priv, W5100_S0_TX_FSR)< skb->len) {
>
> There is a race here.
> Interrupt came come right here before you set the stop_queue()
>
>> + netif_stop_queue(ndev);
>
> So Here I suggest adding a test again
>
> if (w5100_read16(priv, W5100_S0_TX_FSR)>= skb->len)
> netif_wake_queue(ndev);
> else
> return NETDEV_TX_BUSY;
>
>> + return NETDEV_TX_BUSY;
Maybe, but I cannot test this, sorry - when I set breakpoint on this
branch - it never triggered, on all my tests on all boards.
Appears that my processor is too slow and cannot feed packets fast enough.
But will do, maybe it triggers for someone on fastest processors.
--
Mike
^ permalink raw reply
* Re: [PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
From: Eric Dumazet @ 2012-03-31 10:23 UTC (permalink / raw)
To: Indan Zupancic; +Cc: netdev, stable, linux-kernel
In-Reply-To: <40f4b91c5200771f223966f368095c63.squirrel@webmail.greenhost.nl>
On Sat, 2012-03-31 at 20:52 +1100, Indan Zupancic wrote:
> Hello,
>
> Finally, after much searching I found one little bug.
>
> [PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
>
> Small typo resulted in bad code generation for certain
> values of K for the BPF_S_ALU_AND_K instruction.
>
> Signed-off-by: Indan Zupancic <indan@nul.nu>
> ---
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 7c1b765..28bc807 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -289,7 +289,7 @@ void bpf_jit_compile(struct sk_filter *fp)
> EMIT2(0x24, K & 0xFF); /* and imm8,%al */
> } else if (K >= 0xFFFF0000) {
> EMIT2(0x66, 0x25); /* and imm16,%ax */
> - EMIT2(K, 2);
> + EMIT(K, 2);
> } else {
> EMIT1_off32(0x25, K); /* and imm32,%eax */
> }
>
>
Thanks but it was already fixed.
commit 1d24fb3684f347226747c6b11ea426b7b992694e
Author: zhuangfeiran@ict.ac.cn <zhuangfeiran@ict.ac.cn>
Date: Wed Mar 28 23:27:00 2012 +0000
x86 bpf_jit: fix a bug in emitting the 16-bit immediate operand of AND
When K >= 0xFFFF0000, AND needs the two least significant bytes of K as
its operand, but EMIT2() gives it the least significant byte of K and
0x2. EMIT() should be used here to replace EMIT2().
Signed-off-by: Feiran Zhuang <zhuangfeiran@ict.ac.cn>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v5 2/2] Ethernet driver for the WIZnet W5100 chip
From: Eric Dumazet @ 2012-03-31 10:24 UTC (permalink / raw)
To: Mike Sinkovsky; +Cc: netdev, linux-kernel
In-Reply-To: <4F76D3E5.6050605@permonline.ru>
On Sat, 2012-03-31 at 15:52 +0600, Mike Sinkovsky wrote:
> Maybe, but I cannot test this, sorry - when I set breakpoint on this
> branch - it never triggered, on all my tests on all boards.
> Appears that my processor is too slow and cannot feed packets fast enough.
Really ?
A pktgen script should certainly hit your NIC limit, unless this is a
100Gbit link ...
^ permalink raw reply
* [RFC] net: bpf_jit: Two pass JIT and other changes
From: Indan Zupancic @ 2012-03-31 12:12 UTC (permalink / raw)
To: Eric Dumazet, netdev, linux-kernel
Hello,
After fiddling with x86 JIT code I got it to compile the BPF
filters always in two passes instead of looping until the result
is stable. The current code loops at least three times, more
if jump offsets change from far to near because other jumps got
shorter. In the process I simplifying the code a bit, but I
tried to avoid controversial code changes. After the changes
bpf_jit_comp.o is about 1kb smaller.
As I moved the inner loop to a helper function, pretty much all
lines in bpf_jit_comp.c changed and a diff isn't very helpful.
Later changes I can redo for the original version and I'll send
a diff for those separately. But before I go to the trouble of
trying to get a readable patch series for the two pass version,
I'll attach the end result below so people can see if it's worth
the trouble or not.
As I'm currently travelling I've no access to a 64-bit machine,
so everything is done on 32-bit and as yet untested. JITing
should be quite a bit faster with this version, while the
generated code should have pretty much the same speed, as
nothing major changed.
Summary of the changes:
1) Sadly, gcc isn't always smart enough to inline emit_code().
So turn it into a macro to force gcc to inline it. This saves
about 500 bytes. Tested with gcc 4.6.0. An alternative to the
macro would be to create inline emit_code1(), emit_code2() etc.
2) Split EMIT2() in the JMP macro's into a constant bit and a
variable bit. This avoids the need to add them together and
saves about 15 bytes. Adding __builtin_constant_p checks to
emit macros saves more (50 bytes), but is ugly.
3) Only generate a CLEAR_X() when necessary. This is done by having
an extra flag saying if X is used before it is set.
4) Simplify the code by adding a RET 0 as epilogue. This way the
pc_ret0 and cleanup_addr logic can be merged. This was inspired
by tcpdump always generating a RET 0 as last instruction. To make
two pass compilation possible, the pc_ret0 had to go, as it's a
backward jump.
5) Compile in two passes by doing the first pass in reverse.
As all jumps are forward jumps, this way all offsets are known
when needed, making it possible to do just two passes: One to
check if we can JIT the filter and to calculate the code size.
The second pass generates the real code directly into the
allocated memory.
Are these changes worth pursuing? In the end it should speed up
JITing, saves 1kb, doesn't generate worse filter code and I think
it streamlines the code a bit.
Greetings,
Indan
---
Two-pass version of arch/x86/net/bpf_jit_comp.c:
---
/* bpf_jit_comp.c : BPF JIT compiler
*
* Copyright (C) 2011 Eric Dumazet (eric.dumazet@gmail.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; version 2
* of the License.
*/
#include <linux/moduleloader.h>
#include <asm/cacheflush.h>
#include <linux/netdevice.h>
#include <linux/filter.h>
/*
* Conventions :
* EAX : BPF A accumulator
* EBX : BPF X accumulator
* RDI : pointer to skb (first argument given to JIT function)
* RBP : frame pointer (even if CONFIG_FRAME_POINTER=n)
* ECX,EDX,ESI : scratch registers
* r9d : skb->len - skb->data_len (headlen)
* r8 : skb->data
* -8(RBP) : saved RBX value
* -16(RBP)..-80(RBP) : BPF_MEMWORDS values
*/
int bpf_jit_enable __read_mostly;
/*
* assembly code in arch/x86/net/bpf_jit.S
*/
extern u8 sk_load_word[], sk_load_half[], sk_load_byte[], sk_load_byte_msh[];
extern u8 sk_load_word_ind[], sk_load_half_ind[], sk_load_byte_ind[];
#define EMIT(bytes, len) \
({ \
if ((len) == 1) \
*prog = (u8)(bytes); \
else if ((len) == 2) \
*(u16 *)prog = (u16)(bytes); \
else { \
*(u32 *)prog = (bytes); \
} \
prog += (len); \
})
#define EMIT1(b1) EMIT(b1, 1)
#define EMIT2(b1, b2) EMIT((b1) | ((b2) << 8), 2)
#define EMIT3(b1, b2, b3) EMIT((b1) + ((b2) << 8) + ((b3) << 16), 3)
#define EMIT4(b1, b2, b3, b4) EMIT((b1) + ((b2) << 8) + ((b3) << 16) + ((b4) << 24), 4)
#define EMIT1_off32(b1, off) do { EMIT1(b1); EMIT(off, 4);} while (0)
#define CLEAR_A() EMIT2(0x31, 0xc0) /* xor %eax,%eax */
#define CLEAR_X() EMIT2(0x31, 0xdb) /* xor %ebx,%ebx */
static inline bool is_imm8(int value)
{
return value <= 127 && value >= -128;
}
#define EMIT_JMP(offset) \
do { \
if (is_imm8(offset)) { \
EMIT1(0xeb); /* jmp .+off8 */ \
EMIT1(offset); \
} else { \
EMIT1_off32(0xe9, offset); /* jmp .+off32 */ \
} \
} while (0)
/* list of x86 cond jumps opcodes (. + s8)
* Add 0x10 (and an extra 0x0f) to generate far jumps (. + s32)
*/
#define X86_JB 0x72
#define X86_JAE 0x73
#define X86_JE 0x74
#define X86_JNE 0x75
#define X86_JBE 0x76
#define X86_JA 0x77
#define EMIT_COND_JMP(op, offset) \
do { \
if (is_imm8(offset)) { \
EMIT1(op); /* jxx .+off8 */ \
EMIT1(offset); \
} else { \
EMIT2(0x0f, op + 0x10); \
EMIT(offset, 4); /* jxx .+off32 */ \
} \
} while (0)
#define COND_SEL(CODE, TOP, FOP) \
case CODE: \
t_op = TOP; \
f_op = FOP; \
goto cond_branch
#define SKB_OFF(field) offsetof(struct sk_buff, field)
#define SEEN_DATAREF 1 /* might call external helpers */
#define SEEN_MEM 2 /* use mem[] for temporary storage */
#define USE_XREG 4 /* ebx is used */
#define CLEAR_XREG 8 /* ebx needs to be zeroed */
#define SEEN_XREG (USE_XREG|CLEAR_XREG) /* ebx is used as input */
static inline void bpf_flush_icache(void *start, void *end)
{
mm_segment_t old_fs = get_fs();
set_fs(KERNEL_DS);
smp_wmb();
flush_icache_range((unsigned long)start, (unsigned long)end);
set_fs(old_fs);
}
static u8* bpf_jit_ins(u8 *prog, const struct sock_filter *f, u16 *addrs, u8 *flags)
{
int t_offset, f_offset;
u8 t_op, f_op;
u8 *func;
u8 seen = *flags;
unsigned int K = f->k;
switch (f->code) {
case BPF_S_ALU_ADD_X: /* A += X; */
seen |= SEEN_XREG;
EMIT2(0x01, 0xd8); /* add %ebx,%eax */
break;
case BPF_S_ALU_ADD_K: /* A += K; */
if (!K)
break;
if (is_imm8(K))
EMIT3(0x83, 0xc0, K); /* add imm8,%eax */
else
EMIT1_off32(0x05, K); /* add imm32,%eax */
break;
case BPF_S_ALU_SUB_X: /* A -= X; */
seen |= SEEN_XREG;
EMIT2(0x29, 0xd8); /* sub %ebx,%eax */
break;
case BPF_S_ALU_SUB_K: /* A -= K */
if (!K)
break;
if (is_imm8(K))
EMIT3(0x83, 0xe8, K); /* sub imm8,%eax */
else
EMIT1_off32(0x2d, K); /* sub imm32,%eax */
break;
case BPF_S_ALU_MUL_X: /* A *= X; */
seen |= SEEN_XREG;
EMIT3(0x0f, 0xaf, 0xc3); /* imul %ebx,%eax */
break;
case BPF_S_ALU_MUL_K: /* A *= K */
if (is_imm8(K))
EMIT3(0x6b, 0xc0, K); /* imul imm8,%eax,%eax */
else {
EMIT2(0x69, 0xc0); /* imul imm32,%eax */
EMIT(K, 4);
}
break;
case BPF_S_ALU_DIV_X: /* A /= X; */
seen |= SEEN_XREG;
EMIT2(0x85, 0xdb); /* test %ebx,%ebx */
/*
* Add 4 to jump over "xor %edx,%edx; div %ebx".
*/
EMIT_COND_JMP(X86_JE, 4 + addrs[0]);
EMIT4(0x31, 0xd2, 0xf7, 0xf3); /* xor %edx,%edx; div %ebx */
break;
case BPF_S_ALU_DIV_K: /* A = reciprocal_divide(A, K); */
EMIT3(0x48, 0x69, 0xc0); /* imul imm32,%rax,%rax */
EMIT(K, 4);
EMIT4(0x48, 0xc1, 0xe8, 0x20); /* shr $0x20,%rax */
break;
case BPF_S_ALU_AND_X:
seen |= SEEN_XREG;
EMIT2(0x21, 0xd8); /* and %ebx,%eax */
break;
case BPF_S_ALU_AND_K:
if (K >= 0xFFFFFF00) {
EMIT2(0x24, K & 0xFF); /* and imm8,%al */
} else if (K >= 0xFFFF0000) {
EMIT2(0x66, 0x25); /* and imm16,%ax */
EMIT(K, 2);
} else {
EMIT1_off32(0x25, K); /* and imm32,%eax */
}
break;
case BPF_S_ALU_OR_X:
seen |= SEEN_XREG;
EMIT2(0x09, 0xd8); /* or %ebx,%eax */
break;
case BPF_S_ALU_OR_K:
if (is_imm8(K))
EMIT3(0x83, 0xc8, K); /* or imm8,%eax */
else
EMIT1_off32(0x0d, K); /* or imm32,%eax */
break;
case BPF_S_ALU_LSH_X: /* A <<= X; */
seen |= SEEN_XREG;
EMIT4(0x89, 0xd9, 0xd3, 0xe0); /* mov %ebx,%ecx; shl %cl,%eax */
break;
case BPF_S_ALU_LSH_K:
if (K == 0)
break;
else if (K == 1)
EMIT2(0xd1, 0xe0); /* shl %eax */
else
EMIT3(0xc1, 0xe0, K);
break;
case BPF_S_ALU_RSH_X: /* A >>= X; */
seen |= SEEN_XREG;
EMIT4(0x89, 0xd9, 0xd3, 0xe8); /* mov %ebx,%ecx; shr %cl,%eax */
break;
case BPF_S_ALU_RSH_K: /* A >>= K; */
if (K == 0)
break;
else if (K == 1)
EMIT2(0xd1, 0xe8); /* shr %eax */
else
EMIT3(0xc1, 0xe8, K);
break;
case BPF_S_ALU_NEG:
EMIT2(0xf7, 0xd8); /* neg %eax */
break;
case BPF_S_RET_K:
t_offset = addrs[0];
if (!K && t_offset) {
EMIT_JMP(t_offset);
} else {
EMIT1_off32(0xb8, K); /* mov $imm32,%eax */
EMIT_JMP(t_offset + 2);
}
break;
case BPF_S_RET_A:
EMIT_JMP(addrs[0] + 2);
break;
case BPF_S_MISC_TAX: /* X = A */
seen |= USE_XREG;
seen &= ~CLEAR_XREG;
EMIT2(0x89, 0xc3); /* mov %eax,%ebx */
break;
case BPF_S_MISC_TXA: /* A = X */
seen |= SEEN_XREG;
EMIT2(0x89, 0xd8); /* mov %ebx,%eax */
break;
case BPF_S_LD_IMM: /* A = K */
if (!K)
CLEAR_A();
else
EMIT1_off32(0xb8, K); /* mov $imm32,%eax */
break;
case BPF_S_LDX_IMM: /* X = K */
seen |= USE_XREG;
seen &= ~CLEAR_XREG;
if (!K)
CLEAR_X();
else
EMIT1_off32(0xbb, K); /* mov $imm32,%ebx */
break;
case BPF_S_LD_MEM: /* A = mem[K] : mov off8(%rbp),%eax */
seen |= SEEN_MEM;
EMIT3(0x8b, 0x45, 0xf0 - K*4);
break;
case BPF_S_LDX_MEM: /* X = mem[K] : mov off8(%rbp),%ebx */
seen |= USE_XREG | SEEN_MEM;
seen &= ~CLEAR_XREG;
EMIT3(0x8b, 0x5d, 0xf0 - K*4);
break;
case BPF_S_ST: /* mem[K] = A : mov %eax,off8(%rbp) */
seen |= SEEN_MEM;
EMIT3(0x89, 0x45, 0xf0 - K*4);
break;
case BPF_S_STX: /* mem[K] = X : mov %ebx,off8(%rbp) */
seen |= SEEN_XREG | SEEN_MEM;
EMIT3(0x89, 0x5d, 0xf0 - K*4);
break;
case BPF_S_LD_W_LEN: /* A = skb->len; */
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
if (is_imm8(SKB_OFF(len)))
/* mov off8(%rdi),%eax */
EMIT3(0x8b, 0x47, SKB_OFF(len));
else {
EMIT2(0x8b, 0x87);
EMIT(SKB_OFF(len), 4);
}
break;
case BPF_S_LDX_W_LEN: /* X = skb->len; */
seen |= USE_XREG;
seen &= ~CLEAR_XREG;
if (is_imm8(SKB_OFF(len)))
/* mov off8(%rdi),%ebx */
EMIT3(0x8b, 0x5f, SKB_OFF(len));
else {
EMIT2(0x8b, 0x9f);
EMIT(SKB_OFF(len), 4);
}
break;
case BPF_S_ANC_PROTOCOL: /* A = ntohs(skb->protocol); */
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, protocol) != 2);
if (is_imm8(SKB_OFF(protocol))) {
/* movzwl off8(%rdi),%eax */
EMIT4(0x0f, 0xb7, 0x47, SKB_OFF(protocol));
} else {
EMIT3(0x0f, 0xb7, 0x87); /* movzwl off32(%rdi),%eax */
EMIT(SKB_OFF(protocol), 4);
}
EMIT2(0x86, 0xc4); /* ntohs() : xchg %al,%ah */
break;
case BPF_S_ANC_IFINDEX:
if (is_imm8(SKB_OFF(dev))) {
/* movq off8(%rdi),%rax */
EMIT4(0x48, 0x8b, 0x47, SKB_OFF(dev));
} else {
EMIT3(0x48, 0x8b, 0x87); /* movq off32(%rdi),%rax */
EMIT(SKB_OFF(dev), 4);
}
EMIT3(0x48, 0x85, 0xc0); /* test %rax,%rax */
EMIT_COND_JMP(X86_JE, 6 + addrs[0]);
EMIT2(0x8b, 0x80); /* mov off32(%rax),%eax */
EMIT(offsetof(struct net_device, ifindex), 4);
BUILD_BUG_ON(FIELD_SIZEOF(struct net_device, ifindex) != 4);
break;
case BPF_S_ANC_MARK:
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, mark) != 4);
if (is_imm8(SKB_OFF(mark))) {
/* mov off8(%rdi),%eax */
EMIT3(0x8b, 0x47, SKB_OFF(mark));
} else {
EMIT2(0x8b, 0x87);
EMIT(SKB_OFF(mark), 4);
}
break;
case BPF_S_ANC_RXHASH:
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, rxhash) != 4);
if (is_imm8(SKB_OFF(rxhash))) {
/* mov off8(%rdi),%eax */
EMIT3(0x8b, 0x47, SKB_OFF(rxhash));
} else {
EMIT2(0x8b, 0x87);
EMIT(SKB_OFF(rxhash), 4);
}
break;
case BPF_S_ANC_QUEUE:
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, queue_mapping) != 2);
if (is_imm8(SKB_OFF(queue_mapping))) {
/* movzwl off8(%rdi),%eax */
EMIT4(0x0f, 0xb7, 0x47, SKB_OFF(queue_mapping));
} else {
EMIT3(0x0f, 0xb7, 0x87); /* movzwl off32(%rdi),%eax */
EMIT(SKB_OFF(queue_mapping), 4);
}
break;
case BPF_S_ANC_CPU:
#ifdef CONFIG_SMP
EMIT4(0x65, 0x8b, 0x04, 0x25); /* mov %gs:off32,%eax */
EMIT((u32)(unsigned long)&cpu_number, 4); /* A = smp_processor_id(); */
#else
CLEAR_A();
#endif
break;
case BPF_S_LD_W_ABS:
func = sk_load_word;
common_load: seen |= SEEN_DATAREF;
if ((int)K < 0) {
/* Abort the JIT because __load_pointer() is needed. */
return NULL;
}
t_offset = func - (prog + 10);
EMIT1_off32(0xbe, K); /* mov imm32,%esi */
EMIT1_off32(0xe8, t_offset); /* call */
break;
case BPF_S_LD_H_ABS:
func = sk_load_half;
goto common_load;
case BPF_S_LD_B_ABS:
func = sk_load_byte;
goto common_load;
case BPF_S_LDX_B_MSH:
if ((int)K < 0) {
/* Abort the JIT because __load_pointer() is needed. */
return NULL;
}
seen |= SEEN_DATAREF | USE_XREG;
seen &= ~CLEAR_XREG;
t_offset = sk_load_byte_msh - (prog + 10);
EMIT1_off32(0xbe, K); /* mov imm32,%esi */
EMIT1_off32(0xe8, t_offset); /* call sk_load_byte_msh */
break;
case BPF_S_LD_W_IND:
func = sk_load_word_ind;
common_load_ind:
seen |= SEEN_DATAREF | SEEN_XREG;
t_offset = func - (prog + 10);
EMIT1_off32(0xbe, K); /* mov imm32,%esi */
EMIT1_off32(0xe8, t_offset); /* call sk_load_xxx_ind */
break;
case BPF_S_LD_H_IND:
func = sk_load_half_ind;
goto common_load_ind;
case BPF_S_LD_B_IND:
func = sk_load_byte_ind;
goto common_load_ind;
case BPF_S_JMP_JA:
t_offset = addrs[0] - addrs[K];
if (t_offset)
EMIT_JMP(t_offset);
break;
COND_SEL(BPF_S_JMP_JGT_K, X86_JA, X86_JBE);
COND_SEL(BPF_S_JMP_JGE_K, X86_JAE, X86_JB);
COND_SEL(BPF_S_JMP_JEQ_K, X86_JE, X86_JNE);
COND_SEL(BPF_S_JMP_JSET_K,X86_JNE, X86_JE);
COND_SEL(BPF_S_JMP_JGT_X, X86_JA, X86_JBE);
COND_SEL(BPF_S_JMP_JGE_X, X86_JAE, X86_JB);
COND_SEL(BPF_S_JMP_JEQ_X, X86_JE, X86_JNE);
COND_SEL(BPF_S_JMP_JSET_X,X86_JNE, X86_JE);
cond_branch: f_offset = addrs[0] - addrs[f->jf];
t_offset = addrs[0] - addrs[f->jt];
/* same targets, can avoid doing the test :) */
if (f->jt == f->jf) {
EMIT_JMP(t_offset);
break;
}
switch (f->code) {
case BPF_S_JMP_JGT_X:
case BPF_S_JMP_JGE_X:
case BPF_S_JMP_JEQ_X:
seen |= SEEN_XREG;
EMIT2(0x39, 0xd8); /* cmp %ebx,%eax */
break;
case BPF_S_JMP_JSET_X:
seen |= SEEN_XREG;
EMIT2(0x85, 0xd8); /* test %ebx,%eax */
break;
case BPF_S_JMP_JEQ_K:
if (K == 0) {
EMIT2(0x85, 0xc0); /* test %eax,%eax */
break;
}
case BPF_S_JMP_JGT_K:
case BPF_S_JMP_JGE_K:
if (K <= 127)
EMIT3(0x83, 0xf8, K); /* cmp imm8,%eax */
else
EMIT1_off32(0x3d, K); /* cmp imm32,%eax */
break;
case BPF_S_JMP_JSET_K:
if (K <= 0xFF)
EMIT2(0xa8, K); /* test imm8,%al */
else if (!(K & 0xFFFF00FF))
EMIT3(0xf6, 0xc4, K >> 8); /* test imm8,%ah */
else if (K <= 0xFFFF) {
EMIT2(0x66, 0xa9); /* test imm16,%ax */
EMIT(K, 2);
} else {
EMIT1_off32(0xa9, K); /* test imm32,%eax */
}
break;
}
if (t_offset) {
if (f_offset)
t_offset += is_imm8(f_offset) ? 2 : 5;
EMIT_COND_JMP(t_op, t_offset);
if (f_offset)
EMIT_JMP(f_offset);
break;
}
if (f_offset)
EMIT_COND_JMP(f_op, f_offset);
break;
default:
/* hmm, too complex filter, give up with jit compiler */
return NULL;
}
*flags |= seen;
return prog;
}
void bpf_jit_compile(struct sk_filter *fp)
{
u8 temp[64];
u8 *prog;
unsigned int proglen = 0;
u8 seen = 0;
u8 *image;
u16 *addrs;
const struct sock_filter *filter = fp->insns;
int i, flen = fp->len;
if (!bpf_jit_enable)
return;
addrs = kmalloc(flen * sizeof(*addrs), GFP_KERNEL);
if (addrs == NULL)
return;
/*
* First pass: Check if we can JIT this and calculate the size.
*
* As there are only forward jumps, go backwards through the
* instructions so all jump offsets are known when needed.
*/
for (i = flen - 1; i >= 0; i--) {
/*
* Store the offset from the end of this instruction to
* the beginning of the epilogue (RET 0) in addrs[i].
*
* To return zero, jump addrs[i] bytes.
* To return A, jump addrs[i] + 2 bytes, to skip the CLEAR_A.
* To skip N instructions, jump addrs[i] - addrs[i + N] bytes.
*/
addrs[i] = proglen;
prog = bpf_jit_ins(temp, &filter[i], &addrs[i], &seen);
if (!prog)
goto out;
proglen += prog - temp;
}
/* Allocate extra memory for the prologue and epilogue */
proglen += 64;
/* Don't use too much memory */
if (proglen > 32 * 1024)
goto out;
image = module_alloc(max_t(unsigned int, proglen, sizeof(struct work_struct)));
if (!image)
goto out;
/*
* Second pass, now for real.
*/
prog = image;
/* No prologue for trivial filters (RET something) */
if (seen) {
EMIT4(0x55, 0x48, 0x89, 0xe5); /* push %rbp; mov %rsp,%rbp */
EMIT4(0x48, 0x83, 0xec, 96); /* subq $96,%rsp */
/* note : must save %rbx in case bpf_error is hit */
if (seen & (USE_XREG | SEEN_DATAREF))
EMIT4(0x48, 0x89, 0x5d, 0xf8); /* mov %rbx, -8(%rbp) */
if (seen & CLEAR_XREG)
CLEAR_X(); /* make sure we don't leak kernel memory */
/*
* If this filter needs to access skb data,
* loads r9 and r8 with :
* r9 = skb->len - skb->data_len
* r8 = skb->data
*/
if (seen & SEEN_DATAREF) {
if (is_imm8(SKB_OFF(len)))
/* mov off8(%rdi),%r9d */
EMIT4(0x44, 0x8b, 0x4f, SKB_OFF(len));
else {
/* mov off32(%rdi),%r9d */
EMIT3(0x44, 0x8b, 0x8f);
EMIT(SKB_OFF(len), 4);
}
if (is_imm8(SKB_OFF(data_len)))
/* sub off8(%rdi),%r9d */
EMIT4(0x44, 0x2b, 0x4f, SKB_OFF(data_len));
else {
EMIT3(0x44, 0x2b, 0x8f);
EMIT(SKB_OFF(data_len), 4);
}
/* mov off32(%rdi),%r8 */
EMIT3(0x4c, 0x8b, 0x87);
EMIT(SKB_OFF(data), 4);
}
}
switch (filter[0].code) {
case BPF_S_RET_K:
case BPF_S_LD_W_ABS:
case BPF_S_LD_H_ABS:
case BPF_S_LD_B_ABS:
case BPF_S_LD_W_LEN:
case BPF_S_LD_W_IND:
case BPF_S_LD_H_IND:
case BPF_S_LD_B_IND:
case BPF_S_LD_IMM:
case BPF_S_ANC_PROTOCOL:
case BPF_S_ANC_IFINDEX:
case BPF_S_ANC_MARK:
case BPF_S_ANC_QUEUE:
case BPF_S_ANC_RXHASH:
case BPF_S_ANC_CPU:
/* first instruction sets A register (or is RET 'constant') */
break;
default:
/* make sure we don't leak kernel information to user */
CLEAR_A(); /* A = 0 */
}
for (i = 0; i < flen; i++) {
prog = bpf_jit_ins(prog, &filter[i], &addrs[i], &seen);
}
/*
* Epilogue is a RET 0.
* RET A jumps to addr[i] + 2, to skip the A = 0.
*/
CLEAR_A();
if (seen & USE_XREG)
EMIT4(0x48, 0x8b, 0x5d, 0xf8); /* mov -8(%rbp),%rbx */
if (seen)
EMIT1(0xc9); /* leaveq */
EMIT1(0xc3); /* ret */
proglen = prog - image;
if (bpf_jit_enable > 1) {
pr_err("flen=%d proglen=%u image=%p\n", flen, proglen, image);
print_hex_dump(KERN_ERR, "JIT code: ", DUMP_PREFIX_ADDRESS,
16, 1, image, proglen, false);
}
bpf_flush_icache(image, image + proglen);
fp->bpf_func = (void *)image;
out:
kfree(addrs);
return;
}
static void jit_free_defer(struct work_struct *arg)
{
module_free(NULL, arg);
}
/* run from softirq, we must use a work_struct to call
* module_free() from process context
*/
void bpf_jit_free(struct sk_filter *fp)
{
if (fp->bpf_func != sk_run_filter) {
struct work_struct *work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, jit_free_defer);
schedule_work(work);
}
}
^ permalink raw reply
* Re: sendmmsg: put_user vs __put_user
From: Ulrich Drepper @ 2012-03-31 12:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20120330.205120.2221145622131588797.davem@davemloft.net>
On Fri, Mar 30, 2012 at 20:51, David Miller <davem@davemloft.net> wrote:
> Compat processes are not able to generate virtual addresses anywhere
> near the range where the kernel resides, so the address range
> verification done by put_user() is completely superfluous and
> therefore not necessary. The normal exception handling done by the
> access is completely sufficient.
I was more thinking about the effects of might_fault() then additional tests.
^ permalink raw reply
* Re: [PATCH] net: reference the ipv4 sysctl table header
From: Djalal Harouni @ 2012-03-31 14:25 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David S. Miller, Al Viro, netdev, Steven Rostedt
In-Reply-To: <m1sjgtmozu.fsf@fess.ebiederm.org>
On Tue, Mar 27, 2012 at 07:35:33PM -0700, Eric W. Biederman wrote:
> Djalal Harouni <tixxdz@opendz.org> writes:
>
> > On Mon, Mar 26, 2012 at 03:50:30PM -0700, Eric W. Biederman wrote:
[...]
> > Anyway they seem false positive ones, since keeping a reference to
> > sysctl_header as in my previous (ugly) patch will quiet the last two
> > ones.
>
> Ok thanks. If that is what it is. Then clean way to quite this will
> ultimately be converting these table to be compatible with my brand
> new register_sysctl() and using that to register them.
>
> In fact I am pretty certain we can just do:
> register_sysctl("net/ipv4/route", ipv4_route_table);
> register_sysctl("net/ipv4/neigh", empty);
Thanks, I've tested this and it works, however it seems that there are still
memleaks, please see below.
> instead of:
> register_sysctl_paths(ipv4_path, ipv4_skeleton);
>
> And kill ipv4_skeleton and ipv4_path as they are now unused.
Right, and use the path directly from the .data section.
> There was a tremendous cleanup and speed up that came with not allowing
> sysctl tables to support .child entries in the core, and the older
> registration routines break apart the tables and return a compatilibty
> sysctl_table_header if we do that, and I believe we are just
> leaking that compatibility sysctl_table_header.
Thanks for the explanation, however I don't understand all of this since
I'm not familiar with the old/new code, but I want to report some notes
(I'm not sure if they are correct).
1) memleaks:
After testing your proposed change:
register_sysctl("net/ipv4/route", ipv4_route_table);
for ipv4_route_table it is ok, no memleak is reported since the returned
ctl_table_header is referenced by the ctl_node nodes. This will silence
the memleak report.
for:
register_sysctl("net/ipv4/neigh", empty);
Here we'll still have the memleak, I don't know if this is the "leaking
that compatibility sysctl_table_header" you are referring to. But here the
returned ctl_table_header will not be referenced by the ctl_node nodes
since we have an empty ctl_table. see init_header()
So is there real memleaks that got hidden by the false positive ones ?
or perhaps these are real memleaks that were never spotted ?
2) From the __register_sysctl_table(), if the ctl_table is empty then
nr_entries == 0.
later we have:
header = kzalloc(sizeof(struct ctl_table_header) +
sizeof(struct ctl_node)*nr_entries, GFP_KERNEL);
...
node = (struct ctl_node *)(header + 1);
init_header(header, root, set, node, table);
...
node points to somewhere, but its use in this situations seems restricted.
3) Why we always alloc ctl_table_header in __register_sysctl_table() at
the beginning ? if I'm right, the current design is:
"each directory should have its ctl_table_header", so if some ctl_tables
share the same directory then they should also share the same
ctl_table_header ?
In this case the code can be improved, check if it's a new dir, then alloc
a new ctl_table_header, otherwise just return the old one and increment
the counter and insert ctl_table entries there.
I say this since currently doing:
register_sysctl("net/ipv4/neigh", empty);
register_sysctl("net/ipv4/neigh", empty);
register_sysctl("net/ipv4/neigh", empty);
Will allocate three ctl_table_header however at the second call the code
successfully detects that this is _not_ a new dir, but it will return the
ctl_table_header that was allocated at the beginning of
__register_sysctl_table().
Is this also related to the "leaking that compatibility sysctl_table_header" ?
4) Is the order of sysctl locking inside insert_links() correct ?
If I'm missing something, please just point-me to an http-link and I'll try
to take a closer look when time permits, otherwise someone who knows this
should just take a look at it.
Sorry for the delay (just got some free time).
Thanks.
--
tixxdz
http://opendz.org
^ permalink raw reply
* Re: [PATCH] net: reference the ipv4 sysctl table header
From: Djalal Harouni @ 2012-03-31 14:29 UTC (permalink / raw)
To: David Miller; +Cc: rostedt, ebiederm, viro, netdev
In-Reply-To: <20120328.165132.1309449708287640005.davem@davemloft.net>
On Wed, Mar 28, 2012 at 04:51:32PM -0400, David Miller wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> Date: Wed, 28 Mar 2012 12:32:20 -0400
>
> > I just started using kmemleak and notice that it reports false positives
> > for several __init functions that call register_sysctl_paths(). The fix
> > you want is:
> >
> > {
> > sturct ctl_table_header *head;
> >
> > head = register_sysctl_paths(ipv4_path, ipv4_skeleton);
> > BUG_ON(!head);
> > kmemleak_ignore(head);
> >
> >
> > No need to waste a pointer just to keep the reference around for
> > kmemleak.
Thanks Steven, yes it's a nice solution for false positive ones.
> That looks a lot saner than the other suggestions, indeed.
Yes, however now I'm not sure that these are real false positive ones,
please check my other email, and someone who knows the code should take a
closer look.
Thank.
> --
> 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
--
tixxdz
http://opendz.org
^ permalink raw reply
* Re: [PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
From: Greg KH @ 2012-03-31 15:13 UTC (permalink / raw)
To: Indan Zupancic; +Cc: Eric Dumazet, netdev, stable, linux-kernel
In-Reply-To: <40f4b91c5200771f223966f368095c63.squirrel@webmail.greenhost.nl>
On Sat, Mar 31, 2012 at 08:52:09PM +1100, Indan Zupancic wrote:
> Hello,
>
> Finally, after much searching I found one little bug.
>
> [PATCH] net: bpf_jit: fix BPF_S_ALU_AND_K compilation
>
> Small typo resulted in bad code generation for certain
> values of K for the BPF_S_ALU_AND_K instruction.
>
> Signed-off-by: Indan Zupancic <indan@nul.nu>
> ---
>
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>
^ permalink raw reply
* [PATCH iputils] ping,ping6: Fallback to numeric addresses while exiting
From: Sergey Fionov @ 2012-03-31 15:40 UTC (permalink / raw)
To: netdev
When user presses Ctrl-C ping waits for pr_addr() to be finished and
then exits. With poor connection it may result in stuck at
gethostbyaddr().
This patch changes handling of exit signals. After setting "exiting"
flag we longjmp() from gethostbyaddr() and print numeric addresses.
---
ping.c | 7 ++++++-
ping6.c | 6 +++++-
ping_common.c | 4 ++++
ping_common.h | 4 ++++
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/ping.c b/ping.c
index eacb29d..ddce1fe 100644
--- a/ping.c
+++ b/ping.c
@@ -1160,12 +1160,17 @@ pr_addr(__u32 addr)
struct hostent *hp;
static char buf[4096];
- if ((options & F_NUMERIC) ||
+ in_pr_addr = !setjmp(pr_addr_jmp);
+
+ if (exiting || (options & F_NUMERIC) ||
!(hp = gethostbyaddr((char *)&addr, 4, AF_INET)))
sprintf(buf, "%s", inet_ntoa(*(struct in_addr *)&addr));
else
snprintf(buf, sizeof(buf), "%s (%s)", hp->h_name,
inet_ntoa(*(struct in_addr *)&addr));
+
+ in_pr_addr = 0;
+
return(buf);
}
diff --git a/ping6.c b/ping6.c
index c5ff881..f5beaf2 100644
--- a/ping6.c
+++ b/ping6.c
@@ -1538,9 +1538,13 @@ char * pr_addr(struct in6_addr *addr)
{
struct hostent *hp = NULL;
- if (!(options&F_NUMERIC))
+ in_pr_addr = !setjmp(pr_addr_jmp);
+
+ if (!(exiting || options&F_NUMERIC))
hp = gethostbyaddr((__u8*)addr, sizeof(struct in6_addr), AF_INET6);
+ in_pr_addr = 0;
+
return hp ? hp->h_name : pr_addr_n(addr);
}
diff --git a/ping_common.c b/ping_common.c
index 82320b1..b45b5c8 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -30,6 +30,8 @@ struct timeval start_time, cur_time;
volatile int exiting;
volatile int status_snapshot;
int confirm = 0;
+int in_pr_addr = 0; /* pr_addr() is executing */
+jmp_buf pr_addr_jmp;
/* Stupid workarounds for bugs/missing functionality in older linuces.
* confirm_flag fixes refusing service of kernels without MSG_CONFIRM.
@@ -248,6 +250,8 @@ void common_options(int ch)
static void sigexit(int signo)
{
exiting = 1;
+ if (in_pr_addr)
+ longjmp(pr_addr_jmp, 0);
}
static void sigstatus(int signo)
diff --git a/ping_common.h b/ping_common.h
index 730cf9b..2262f91 100644
--- a/ping_common.h
+++ b/ping_common.h
@@ -16,6 +16,7 @@
#include <errno.h>
#include <string.h>
#include <netdb.h>
+#include <setjmp.h>
#include <netinet/in.h>
#include <arpa/inet.h>
@@ -206,3 +207,6 @@ extern int gather_statistics(__u8 *ptr, int icmplen,
int csfailed, struct timeval *tv, char *from,
void (*pr_reply)(__u8 *ptr, int cc));
extern void print_timestamp(void);
+
+extern int in_pr_addr;
+extern jmp_buf pr_addr_jmp;
--
1.7.8.5
^ permalink raw reply related
* ipv6: tunnel: hang when destroying ipv6 tunnel
From: Sasha Levin @ 2012-03-31 17:51 UTC (permalink / raw)
To: davem, kuznet, jmorris, yoshfuji, Patrick McHardy
Cc: netdev, linux-kernel@vger.kernel.org List, Dave Jones
Hi all,
It appears that a hang may occur when destroying an ipv6 tunnel, which
I've reproduced several times in a KVM vm.
The pattern in the stack dump below is consistent with unregistering a
kobject when holding multiple locks. Unregistering a kobject usually
leads to an exit back to userspace with call_usermodehelper_exec().
The userspace code may access sysfs files which in turn will require
locking within the kernel, leading to a deadlock since those locks are
already held by kernel.
[ 1561.564172] INFO: task kworker/u:2:3140 blocked for more than 120 seconds.
[ 1561.566945] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 1561.570062] kworker/u:2 D ffff88006ee63000 4504 3140 2 0x00000000
[ 1561.572968] ffff88006ed9f7e0 0000000000000082 ffff88006ed9f790
ffffffff8107d346
[ 1561.575680] ffff88006ed9ffd8 00000000001d4580 ffff88006ed9e010
00000000001d4580
[ 1561.578601] 00000000001d4580 00000000001d4580 ffff88006ed9ffd8
00000000001d4580
[ 1561.581697] Call Trace:
[ 1561.582650] [<ffffffff8107d346>] ? kvm_clock_read+0x46/0x80
[ 1561.584543] [<ffffffff827063d4>] schedule+0x24/0x70
[ 1561.586231] [<ffffffff82704025>] schedule_timeout+0x245/0x2c0
[ 1561.588508] [<ffffffff81117c9a>] ? mark_held_locks+0x7a/0x120
[ 1561.590858] [<ffffffff81119bbd>] ? __lock_release+0x8d/0x1d0
[ 1561.593162] [<ffffffff82707e6b>] ? _raw_spin_unlock_irq+0x2b/0x70
[ 1561.595394] [<ffffffff810e36d1>] ? get_parent_ip+0x11/0x50
[ 1561.597403] [<ffffffff82705919>] wait_for_common+0x119/0x190
[ 1561.599707] [<ffffffff810ed1b0>] ? try_to_wake_up+0x2c0/0x2c0
[ 1561.601758] [<ffffffff82705a38>] wait_for_completion+0x18/0x20
[ 1561.603843] [<ffffffff810cdcd8>] call_usermodehelper_exec+0x228/0x240
[ 1561.606059] [<ffffffff82705844>] ? wait_for_common+0x44/0x190
[ 1561.608352] [<ffffffff81878445>] kobject_uevent_env+0x615/0x650
[ 1561.610908] [<ffffffff810e36d1>] ? get_parent_ip+0x11/0x50
[ 1561.613146] [<ffffffff8187848b>] kobject_uevent+0xb/0x10
[ 1561.615312] [<ffffffff81876f5a>] kobject_cleanup+0xca/0x1b0
[ 1561.617509] [<ffffffff8187704d>] kobject_release+0xd/0x10
[ 1561.619334] [<ffffffff81876d9c>] kobject_put+0x2c/0x60
[ 1561.621117] [<ffffffff8226ea80>] net_rx_queue_update_kobjects+0xa0/0xf0
[ 1561.623421] [<ffffffff8226ec87>] netdev_unregister_kobject+0x37/0x70
[ 1561.625979] [<ffffffff82253e26>] rollback_registered_many+0x186/0x260
[ 1561.628526] [<ffffffff82253f14>] unregister_netdevice_many+0x14/0x60
[ 1561.631064] [<ffffffff8243922e>] ip6_tnl_destroy_tunnels+0xee/0x160
[ 1561.633549] [<ffffffff8243b8f3>] ip6_tnl_exit_net+0xd3/0x1c0
[ 1561.635843] [<ffffffff8243b820>] ? ip6_tnl_ioctl+0x550/0x550
[ 1561.637972] [<ffffffff81259c86>] ? proc_net_remove+0x16/0x20
[ 1561.639881] [<ffffffff8224f119>] ops_exit_list+0x39/0x60
[ 1561.641666] [<ffffffff8224f72b>] cleanup_net+0xfb/0x1a0
[ 1561.643528] [<ffffffff810ce97d>] process_one_work+0x1cd/0x460
[ 1561.645828] [<ffffffff810ce91c>] ? process_one_work+0x16c/0x460
[ 1561.648180] [<ffffffff8224f630>] ? net_drop_ns+0x40/0x40
[ 1561.650285] [<ffffffff810d1e76>] worker_thread+0x176/0x3b0
[ 1561.652460] [<ffffffff810d1d00>] ? manage_workers+0x120/0x120
[ 1561.654734] [<ffffffff810d727e>] kthread+0xbe/0xd0
[ 1561.656656] [<ffffffff8270a134>] kernel_thread_helper+0x4/0x10
[ 1561.658881] [<ffffffff810e3fe0>] ? finish_task_switch+0x80/0x110
[ 1561.660828] [<ffffffff82708434>] ? retint_restore_args+0x13/0x13
[ 1561.662795] [<ffffffff810d71c0>] ? __init_kthread_worker+0x70/0x70
[ 1561.664932] [<ffffffff8270a130>] ? gs_change+0x13/0x13
[ 1561.667001] 4 locks held by kworker/u:2/3140:
[ 1561.667599] #0: (netns){.+.+.+}, at: [<ffffffff810ce91c>]
process_one_work+0x16c/0x460
[ 1561.668758] #1: (net_cleanup_work){+.+.+.}, at:
[<ffffffff810ce91c>] process_one_work+0x16c/0x460
[ 1561.670002] #2: (net_mutex){+.+.+.}, at: [<ffffffff8224f6b0>]
cleanup_net+0x80/0x1a0
[ 1561.671700] #3: (rtnl_mutex){+.+.+.}, at: [<ffffffff82267f02>]
rtnl_lock+0x12/0x20
^ permalink raw reply
* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: Will Drewry @ 2012-03-31 18:14 UTC (permalink / raw)
To: Vladimir Murzin
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, akpm, corbet, eric.dumazet, markus,
coreyb, keescook, jmorris
In-Reply-To: <20120331044030.GB3077@pinguin>
2012/3/30 Vladimir Murzin <murzin.v@gmail.com>:
> Hi Will,
>
> On Thu, Mar 29, 2012 at 03:01:53PM -0500, Will Drewry wrote:
>
> snipped
>
>> +
>> +/* Limit any path through the tree to 256KB worth of instructions. */
>> +#define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))
>> +
>> +static void seccomp_filter_log_failure(int syscall)
>> +{
>> + int compat = 0;
>> +#ifdef CONFIG_COMPAT
>> + compat = is_compat_task();
>> +#endif
>> + pr_info("%s[%d]: %ssystem call %d blocked at 0x%lx\n",
>> + current->comm, task_pid_nr(current),
>> + (compat ? "compat " : ""),
>> + syscall, KSTK_EIP(current));
>> +}
>
> snipped
>
>> +/**
>> + * seccomp_attach_user_filter - attaches a user-supplied sock_fprog
>> + * @user_filter: pointer to the user data containing a sock_fprog.
>> + *
>> + * Returns 0 on success and non-zero otherwise.
>> + */
>> +long seccomp_attach_user_filter(char __user *user_filter)
>> +{
>> + struct sock_fprog fprog;
>> + long ret = -EFAULT;
>> +
>> +#ifdef CONFIG_COMPAT
>> + if (is_compat_task()) {
>> + struct compat_sock_fprog fprog32;
>> + if (copy_from_user(&fprog32, user_filter, sizeof(fprog32)))
>> + goto out;
>> + fprog.len = fprog32.len;
>> + fprog.filter = compat_ptr(fprog32.filter);
>> + } else /* falls through to the if below. */
>> +#endif
>> + if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
>> + goto out;
>> + ret = seccomp_attach_filter(&fprog);
>> +out:
>> + return ret;
>> +}
>
> Do we really need to surround is_compat_task() with CNFIG_COMPAT?
> It seems that this case has already handled in compat.h [1]
>
> [1] http://lxr.linux.no/#linux+v3.3/include/linux/compat.h#L566
In log_failure, it's not needed at all, but that function disappears
in a subsequent patch in the series that converts over to using the
auditing framework. For the next use of CONFIG_COMPAT, it is more
important for guarding the compat_sock_fprog struct (which isn't
doubly defined) rather than the is_compat_task.
Thanks!
will
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox