* Re: [PATCH net] netpoll: linearize skb before accessing its data
From: Antonio Quartulli @ 2013-10-22 6:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20131021.182319.625146263287554088.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]
On Mon, Oct 21, 2013 at 06:23:19PM -0400, David Miller wrote:
> From: Antonio Quartulli <antonio@meshcoding.com>
> Date: Mon, 21 Oct 2013 23:31:20 +0200
>
> > __netpoll_rx() assumes that the data buffer of the received
> > skb is linear and then passes it to rx_hook().
> > However this is not true because the skb has not been
> > linearized yet.
> >
> > This can cause rx_hook() to access non allocated memory
> > while parsing the received data.
> >
> > Fix __netpoll_rx() by explicitly linearising the skb.
> >
> > Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
>
> It is rx_hook's obligation to access the SKB properly and not
> assume that the SKB is linear. It is very expensive to
> linearize every SKB just for the sake of improperly implemented
> receive hooks.
>
> In particular the rx hooks must make use of interface such
> as pskb_may_pull(), just like every other protocol does
> on packet input processing, to make sure the area they want
> to access is in the linear area.
>
But rx_hook() does not receive any skb:
609 np->rx_hook(np, ntohs(uh->source),
610 (char *)(uh+1),
611 ulen - sizeof(struct udphdr));
it just receives a pointer to the data and can't do anything to make it linear.
(uh is a pointer to the udp header). Am I missing something?
Regards,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 00/15] net: ethernet: remove unnecessary pci_set_drvdata() part 3
From: David Miller @ 2013-10-22 6:13 UTC (permalink / raw)
To: jg1.han; +Cc: netdev
In-Reply-To: <000601cecedd$1981e300$4c85a900$%han@samsung.com>
From: Jingoo Han <jg1.han@samsung.com>
Date: Tue, 22 Oct 2013 13:13:44 +0900
> Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
> (device-core: Ensure drvdata = NULL when no driver is bound),
> the driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/1] [PATCH resubmit] ax88179_178a: Correct the RX error definition in RX header
From: David Miller @ 2013-10-22 6:15 UTC (permalink / raw)
To: freddy; +Cc: linux-usb, linux-kernel, netdev, allan, louis
In-Reply-To: <1382406449-2041-1-git-send-email-freddy@asix.com.tw>
Submit these, not individually as single patches, but as a patch set,
the first one with subject "[PATCH 1/2]" and the second with
subject "[PATCH 2/2]"
You have to do this, because it is absolutely essentially to let
me know which patch gets applies first and which one gets applied
second. The two patches are to the same file, so if you don't
tell me the order, they won't apply cleanly at all.
^ permalink raw reply
* Re: [PATCH net-next] net: remove function sk_reset_txq()
From: David Miller @ 2013-10-22 6:18 UTC (permalink / raw)
To: gamerh2o; +Cc: netdev, linux-kernel
In-Reply-To: <20131022041124.GA14122@will>
From: ZHAO Gang <gamerh2o@gmail.com>
Date: Tue, 22 Oct 2013 12:11:24 +0800
> What sk_reset_txq() does is just calls function sk_tx_queue_reset(),
> and sk_reset_txq() is used only in sock.h, by dst_negative_advice().
> Let dst_negative_advice() calls sk_tx_queue_reset() directly so we
> can remove unneeded sk_reset_txq().
>
> Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>
Still doesn't apply.
Email this patch to yourself, I bet the patch you receive won't
apply cleanly. Likely your email client is corrupting the patch.
You've already eaten enough of my time as a maintainer submitting
a patch that won't even apply. Absolutely do not resubmit this
patch until you can email the patch successfully to yourself
and successfuly apply the patch you receive in that email to
the current net-next tree.
^ permalink raw reply
* Re: [PATCH 1/3] net: cxgb4vf: use DEFINE_PCI_DEVICE_TABLE
From: David Miller @ 2013-10-22 6:19 UTC (permalink / raw)
To: jg1.han; +Cc: netdev, leedom
In-Reply-To: <000e01ceced2$a9e5d970$fdb18c50$%han@samsung.com>
From: Jingoo Han <jg1.han@samsung.com>
Date: Tue, 22 Oct 2013 11:59:02 +0900
> This macro is used to create a struct pci_device_id array.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3] net: tulip: use DEFINE_PCI_DEVICE_TABLE
From: David Miller @ 2013-10-22 6:19 UTC (permalink / raw)
To: jg1.han; +Cc: netdev, grundler
In-Reply-To: <000f01ceced2$dde772b0$99b65810$%han@samsung.com>
From: Jingoo Han <jg1.han@samsung.com>
Date: Tue, 22 Oct 2013 12:00:30 +0900
> This macro is used to create a struct pci_device_id array.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/3] net: ksz884x: use DEFINE_PCI_DEVICE_TABLE
From: David Miller @ 2013-10-22 6:19 UTC (permalink / raw)
To: jg1.han; +Cc: netdev, buytenh
In-Reply-To: <001001ceced3$0bbdea20$2339be60$%han@samsung.com>
From: Jingoo Han <jg1.han@samsung.com>
Date: Tue, 22 Oct 2013 12:01:47 +0900
> This macro is used to create a struct pci_device_id array.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] netpoll: linearize skb before accessing its data
From: David Miller @ 2013-10-22 6:25 UTC (permalink / raw)
To: antonio; +Cc: netdev
In-Reply-To: <20131022060635.GF1544@neomailbox.net>
From: Antonio Quartulli <antonio@meshcoding.com>
Date: Tue, 22 Oct 2013 08:06:35 +0200
> But rx_hook() does not receive any skb:
>
> 609 np->rx_hook(np, ntohs(uh->source),
> 610 (char *)(uh+1),
> 611 ulen - sizeof(struct udphdr));
>
> it just receives a pointer to the data and can't do anything to make it linear.
> (uh is a pointer to the udp header). Am I missing something?
Then this hook's API needs to be fixed, it's completely broken.
Make it pass the SKB and the appropriate offset (from skb->data) in
bytes.
^ permalink raw reply
* Re: [PATCH net] netpoll: linearize skb before accessing its data
From: Antonio Quartulli @ 2013-10-22 6:37 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20131022.022525.2068838304168924147.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
On Tue, Oct 22, 2013 at 02:25:25AM -0400, David Miller wrote:
> From: Antonio Quartulli <antonio@meshcoding.com>
> Date: Tue, 22 Oct 2013 08:06:35 +0200
>
> > But rx_hook() does not receive any skb:
> >
> > 609 np->rx_hook(np, ntohs(uh->source),
> > 610 (char *)(uh+1),
> > 611 ulen - sizeof(struct udphdr));
> >
> > it just receives a pointer to the data and can't do anything to make it linear.
> > (uh is a pointer to the udp header). Am I missing something?
>
> Then this hook's API needs to be fixed, it's completely broken.
>
> Make it pass the SKB and the appropriate offset (from skb->data) in
> bytes.
>
Ok I will do that. But I think this API change must go into net-next, right?
Otherwise we would break every existing user.
What about resending this patch to net (after having removed the pskb_may_pull()
as pointed out by Eric) and then fixing the API in net-next?
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net] netpoll: linearize skb before accessing its data
From: David Miller @ 2013-10-22 6:50 UTC (permalink / raw)
To: antonio; +Cc: netdev
In-Reply-To: <20131022063714.GI1544@neomailbox.net>
From: Antonio Quartulli <antonio@meshcoding.com>
Date: Tue, 22 Oct 2013 08:37:14 +0200
> Ok I will do that. But I think this API change must go into
> net-next, right? Otherwise we would break every existing user.
Too bad, it's a necessary API change to fix the bug, the users
will need to change too.
It's not our problem if they are out-of-tree and can't be fixed
in-situ.
^ permalink raw reply
* Re: [PATCH] 3c59x: fix incorrect use of spin_lock_bh in interrupts
From: David Miller @ 2013-10-22 6:55 UTC (permalink / raw)
To: mpatocka; +Cc: klassert, netdev
In-Reply-To: <alpine.LRH.2.02.1310211950360.5325@file01.intranet.prod.int.rdu2.redhat.com>
From: Mikulas Patocka <mpatocka@redhat.com>
Date: Mon, 21 Oct 2013 19:53:22 -0400 (EDT)
> The functions mdio_read and mdio_write may be called from interrupt
> context. Consequently, we must use spin_lock_irqsave instead of
> spin_lock_bh.
>
> This patch should be backported to stable kernels.
vortex_down() does a lot of other things which are really dangerous
from an interrupt handler, such as del_timer_sync().
The real fix for this bug is to defer the vortex_error() work into
a workqueue, and thus process context, like every other driver does.
^ permalink raw reply
* RE: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Ni, Xun @ 2013-10-22 7:15 UTC (permalink / raw)
To: Daniel Borkmann, Daniel Wagner
Cc: Eric W. Biederman, pablo@netfilter.org,
netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
Tejun Heo, cgroups@vger.kernel.org
In-Reply-To: <52654CE6.7030706@redhat.com>
Hello, Daniel:
can all your examples block early before doing network operations? What's the whole netfilter universe? Can you give us more clear examples?
Thanks
On 10/21/2013 05:09 PM, Daniel Wagner wrote:
> On 10/19/2013 08:16 AM, Daniel Borkmann wrote:
>> On 10/19/2013 01:21 AM, Eric W. Biederman wrote:
>>
>>> I am coming to this late. But two concrete suggestions.
>>>
>>> 1) process groups and sessions don't change as frequently as pids.
>>>
>>> 2) It is possible to put a set of processes in their own network
>>> namespace and pipe just the packets you want those processes to
>>> use into that network namespace. Using an ingress queueing filter
>>> makes that process very efficient even if you have to filter by port.
>>
>> Actually in our case we're filtering outgoing traffic, based on which
>> local socket that originated from; so you wouldn't need all of that
>> construct. Also, you wouldn't even need to have an a-prio knowledge
>> of the application internals regarding their use of particular use of
>> ports or protocols. I don't think that such a setup will have the
>> same efficiency, ease of use, and power to distinguish the
>> application the traffic came from in such a lightweight, protocol independent and easy way.
>
> Sorry for beeing late as well (and also stupid question)
>
> Couldn't you use something from the LSM? I mean you allow the
> application to create the socket etc and then block later the traffic
> originated from that socket. Wouldn't it make more sense to block
> early?
I gave one simple example for blocking in the commit message, that's true, but it is not limited to that, meaning we can have much different scenarios/policies that netfilter allows us than just blocking, e.g. fine grained settings where applications are allowed to connect/send traffic to, application traffic marking/ conntracking, application-specific packet mangling, and so on, just think of the whole netfilter universe.
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] [PATCH] ax88179_178a: Correct the RX error definition in RX header
From: freddy @ 2013-10-22 7:32 UTC (permalink / raw)
To: davem, linux-usb, linux-kernel, netdev, allan, louis; +Cc: Freddy Xin
From: Freddy Xin <freddy@asix.com.tw>
Correct the definition of AX_RXHDR_CRC_ERR and
AX_RXHDR_DROP_ERR. They are BIT29 and BIT31 in pkt_hdr
seperately.
Signed-off-by: Freddy Xin <freddy@asix.com.tw>
---
drivers/net/usb/ax88179_178a.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 3569293..3bcd0d9 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -36,8 +36,8 @@
#define AX_RXHDR_L4_TYPE_TCP 16
#define AX_RXHDR_L3CSUM_ERR 2
#define AX_RXHDR_L4CSUM_ERR 1
-#define AX_RXHDR_CRC_ERR ((u32)BIT(31))
-#define AX_RXHDR_DROP_ERR ((u32)BIT(30))
+#define AX_RXHDR_CRC_ERR ((u32)BIT(29))
+#define AX_RXHDR_DROP_ERR ((u32)BIT(31))
#define AX_ACCESS_MAC 0x01
#define AX_ACCESS_PHY 0x02
#define AX_ACCESS_EEPROM 0x04
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/2] [PATCH] ax88179_178a: Add VID:DID for Samsung USB Ethernet Adapter
From: freddy @ 2013-10-22 7:32 UTC (permalink / raw)
To: davem, linux-usb, linux-kernel, netdev, allan, louis; +Cc: Freddy Xin
In-Reply-To: <1382427131-2429-1-git-send-email-freddy@asix.com.tw>
From: Freddy Xin <freddy@asix.com.tw>
Add VID:DID for Samsung USB Ethernet Adapter.
Signed-off-by: Freddy Xin <freddy@asix.com.tw>
---
drivers/net/usb/ax88179_178a.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 3bcd0d9..846cc19 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1406,6 +1406,19 @@ static const struct driver_info sitecom_info = {
.tx_fixup = ax88179_tx_fixup,
};
+static const struct driver_info samsung_info = {
+ .description = "Samsung USB Ethernet Adapter",
+ .bind = ax88179_bind,
+ .unbind = ax88179_unbind,
+ .status = ax88179_status,
+ .link_reset = ax88179_link_reset,
+ .reset = ax88179_reset,
+ .stop = ax88179_stop,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .rx_fixup = ax88179_rx_fixup,
+ .tx_fixup = ax88179_tx_fixup,
+};
+
static const struct usb_device_id products[] = {
{
/* ASIX AX88179 10/100/1000 */
@@ -1418,7 +1431,11 @@ static const struct usb_device_id products[] = {
}, {
/* Sitecom USB 3.0 to Gigabit Adapter */
USB_DEVICE(0x0df6, 0x0072),
- .driver_info = (unsigned long) &sitecom_info,
+ .driver_info = (unsigned long)&sitecom_info,
+}, {
+ /* Samsung USB Ethernet Adapter */
+ USB_DEVICE(0x04e8, 0xa100),
+ .driver_info = (unsigned long)&samsung_info,
},
{ },
};
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net 1/2] sit: allow to use rtnl ops on fb tunnel
From: Nicolas Dichtel @ 2013-10-22 7:34 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David Miller, netdev, steffen.klassert, pshelar, gregkh, stable
In-Reply-To: <CA+FuTScA26dW8bO5YdKTKp9tjSM-xVqkdkq8xzu-wH0cT=Wh8Q@mail.gmail.com>
Le 22/10/2013 01:30, Willem de Bruijn a écrit :
> On Wed, Oct 2, 2013 at 3:15 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Le 01/10/2013 18:59, David Miller a écrit :
>>
>>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>> Date: Tue, 1 Oct 2013 18:04:59 +0200
>>>
>>>> rtnl ops where introduced by ba3e3f50a0e5 ("sit: advertise tunnel param
>>>> via
>>>> rtnl"), but I forget to assign rtnl ops to fb tunnels.
>>>>
>>>> Now that it is done, we must remove the explicit call to
>>>> unregister_netdevice_queue(), because the fallback tunnel is added to
>>>> the queue
>>>> in sit_destroy_tunnels() when checking rtnl_link_ops of all netdevices
>>>> (this
>>>> is valid since commit 5e6700b3bf98 ("sit: add support of x-netns")).
>>>>
>>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>>
>>>
>>> Applied and queued up for -stable.
>>>
>>> But I imagine since the x-netns changes aren't in various -stable
>>> branches this will need to be adjusted a bit?
>>
>> Yes, it's what I've tried to say in the commit log ;-)
>>
>> In fact, before the x-netns changes, we must keep the
>> unregister_netdevice_queue() line.
>
> In 3.11 linux-stable, this patch was merged between 3.11.4 and 3.11.5
> in commit 3783100, after the x-netns changes in commit 5e6700b3bf, but
> the unregister_netdevice_queue was kept.
>
> I think that caused the following bug. In 3.11.6, a simple `modprobe
> sit && rmmod sit` hits the BUG at net/core/dev.c:5039:
>
> BUG_ON(dev->reg_state != NETREG_REGISTERED);
>
> The device is actually NETREG_RELEASED at one point because the device
> is now unregistered twice. The issue goes away by porting the
> remainder of the original commit: the one liner that removes the call
> to unregister_netdevice_queue.
>
> +++ b/net/ipv6/sit.c
> @@ -1708,7 +1708,6 @@ static void __net_exit sit_exit_net(struct net *net)
>
> sit_destroy_tunnels(sitn, &list);
> - unregister_netdevice_queue(sitn->fb_tunnel_dev, &list);
> unregister_netdevice_many(&list);
>
> If correct, let me know if I should send a proper one-line patch
> against 3.11.y. Since I haven't looked at this code before, I found it
> safer to report the issue first.
Yes, this line should be removed, like it was done in the original patch
(x-netns for sit is part of 3.11).
>
> 5e6700b3bf was not applied to 3.10 stable, so that branch is not affected.
Right.
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Daniel Wagner @ 2013-10-22 7:36 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Eric W. Biederman, pablo-Cap9r6Oaw4JrovVCs/uTlw,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52654CE6.7030706-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 10/21/2013 04:48 PM, Daniel Borkmann wrote:
> On 10/21/2013 05:09 PM, Daniel Wagner wrote:
>> On 10/19/2013 08:16 AM, Daniel Borkmann wrote:
>>> On 10/19/2013 01:21 AM, Eric W. Biederman wrote:
>>>
>>>> I am coming to this late. But two concrete suggestions.
>>>>
>>>> 1) process groups and sessions don't change as frequently as pids.
>>>>
>>>> 2) It is possible to put a set of processes in their own network
>>>> namespace and pipe just the packets you want those processes to
>>>> use into that network namespace. Using an ingress queueing filter
>>>> makes that process very efficient even if you have to filter by
>>>> port.
>>>
>>> Actually in our case we're filtering outgoing traffic, based on which
>>> local socket that originated from; so you wouldn't need all of that
>>> construct. Also, you wouldn't even need to have an a-prio knowledge of
>>> the application internals regarding their use of particular use of ports
>>> or protocols. I don't think that such a setup will have the same
>>> efficiency, ease of use, and power to distinguish the application the
>>> traffic came from in such a lightweight, protocol independent and
>>> easy way.
>>
>> Sorry for beeing late as well (and also stupid question)
>>
>> Couldn't you use something from the LSM? I mean you allow the
>> application to create the socket etc and then block later
>> the traffic originated from that socket. Wouldn't it make
>> more sense to block early?
>
> I gave one simple example for blocking in the commit message,
> that's true, but it is not limited to that, meaning we can have
> much different scenarios/policies that netfilter allows us than
> just blocking, e.g. fine grained settings where applications are
> allowed to connect/send traffic to, application traffic marking/
> conntracking, application-specific packet mangling, and so on,
> just think of the whole netfilter universe.
Oh, I didn't pay enough attention to the commit message. Sorry
about that. Obviously, if fine grained settings is a must
then blocking the write is not good enough.
cheers,
daniel
^ permalink raw reply
* Re: [PATCH net-next v2] bonding: move bond-specific init after enslave happens
From: Ding Tianhong @ 2013-10-22 7:38 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, jiri, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1382348910-32724-1-git-send-email-vfalico@redhat.com>
On 2013/10/21 17:48, Veaceslav Falico wrote:
> As Jiri noted, currently we first do all bonding-specific initialization
> (specifically - bond_select_active_slave(bond)) before we actually attach
> the slave (so that it becomes visible through bond_for_each_slave() and
> friends). This might result in bond_select_active_slave() not seeing the
> first/new slave and, thus, not actually selecting an active slave.
>
> Fix this by moving all the bond-related init part after we've actually
> completely initialized and linked (via bond_master_upper_dev_link()) the
> new slave.
>
> Also, remove the bond_(de/a)ttach_slave(), it's useless to have functions
> to ++/-- one int.
>
> After this we have all the initialization of the new slave *before*
> linking, and all the stuff that needs to be done on bonding *after* it. It
> has also a bonus effect - we can remove the locking on the new slave init
> completely, and only use it for bond_select_active_slave().
>
> Reported-by: Jiri Pirko <jiri@resnulli.us>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
>
> Notes:
> v1 -> v2:
> Move the bond_(de/a)ttach_slave() functionality, and remove these
> functions.
>
> drivers/net/bonding/bond_main.c | 65 +++++++++--------------------------------
> 1 file changed, 14 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index d90734f..2daa066 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -967,33 +967,6 @@ void bond_select_active_slave(struct bonding *bond)
> }
> }
>
> -/*--------------------------- slave list handling ---------------------------*/
> -
> -/*
> - * This function attaches the slave to the end of list.
> - *
> - * bond->lock held for writing by caller.
> - */
> -static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
> -{
> - bond->slave_cnt++;
> -}
> -
> -/*
> - * This function detaches the slave from the list.
> - * WARNING: no check is made to verify if the slave effectively
> - * belongs to <bond>.
> - * Nothing is freed on return, structures are just unchained.
> - * If any slave pointer in bond was pointing to <slave>,
> - * it should be changed by the calling function.
> - *
> - * bond->lock held for writing by caller.
> - */
> -static void bond_detach_slave(struct bonding *bond, struct slave *slave)
> -{
> - bond->slave_cnt--;
> -}
> -
> #ifdef CONFIG_NET_POLL_CONTROLLER
> static inline int slave_enable_netpoll(struct slave *slave)
> {
> @@ -1471,22 +1444,13 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> goto err_close;
> }
>
> - write_lock_bh(&bond->lock);
> -
> prev_slave = bond_last_slave(bond);
> - bond_attach_slave(bond, new_slave);
>
> new_slave->delay = 0;
> new_slave->link_failure_count = 0;
>
> - write_unlock_bh(&bond->lock);
> -
> - bond_compute_features(bond);
> -
> bond_update_speed_duplex(new_slave);
>
> - read_lock(&bond->lock);
> -
> new_slave->last_arp_rx = jiffies -
> (msecs_to_jiffies(bond->params.arp_interval) + 1);
> for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
> @@ -1547,12 +1511,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> }
> }
>
> - write_lock_bh(&bond->curr_slave_lock);
> -
> switch (bond->params.mode) {
> case BOND_MODE_ACTIVEBACKUP:
> bond_set_slave_inactive_flags(new_slave);
> - bond_select_active_slave(bond);
> break;
> case BOND_MODE_8023AD:
> /* in 802.3ad mode, the internal mechanism
> @@ -1578,7 +1539,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> case BOND_MODE_ALB:
> bond_set_active_slave(new_slave);
> bond_set_slave_inactive_flags(new_slave);
> - bond_select_active_slave(bond);
> break;
> default:
> pr_debug("This slave is always active in trunk mode\n");
> @@ -1596,10 +1556,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> break;
> } /* switch(bond_mode) */
>
> - write_unlock_bh(&bond->curr_slave_lock);
> -
> - bond_set_carrier(bond);
> -
> #ifdef CONFIG_NET_POLL_CONTROLLER
> slave_dev->npinfo = bond->dev->npinfo;
> if (slave_dev->npinfo) {
> @@ -1614,8 +1570,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> }
> #endif
>
> - read_unlock(&bond->lock);
> -
> res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
> new_slave);
> if (res) {
> @@ -1629,6 +1583,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> goto err_unregister;
> }
>
> + bond->slave_cnt++;
> + bond_compute_features(bond);
> + bond_set_carrier(bond);
> +
> + if (USES_PRIMARY(bond->params.mode)) {
> + read_lock(&bond->lock);
> + write_lock_bh(&bond->curr_slave_lock);
> + bond_select_active_slave(bond);
> + write_unlock_bh(&bond->curr_slave_lock);
> + read_unlock(&bond->lock);
> + }
>
> pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
> bond_dev->name, slave_dev->name,
> @@ -1648,7 +1613,6 @@ err_detach:
>
> vlan_vids_del_by_dev(slave_dev, bond_dev);
> write_lock_bh(&bond->lock);
> - bond_detach_slave(bond, new_slave);
> if (bond->primary_slave == new_slave)
> bond->primary_slave = NULL;
> if (bond->curr_active_slave == new_slave) {
> @@ -1686,7 +1650,6 @@ err_free:
> kfree(new_slave);
>
> err_undo_flags:
> - bond_compute_features(bond);
> /* Enslave of first slave has failed and we need to fix master's mac */
> if (!bond_has_slaves(bond) &&
> ether_addr_equal(bond_dev->dev_addr, slave_dev->dev_addr))
> @@ -1740,6 +1703,9 @@ static int __bond_release_one(struct net_device *bond_dev,
>
> write_unlock_bh(&bond->lock);
>
> + /* release the slave from its bond */
> + bond->slave_cnt--;
> +
> bond_upper_dev_unlink(bond_dev, slave_dev);
> /* unregister rx_handler early so bond_handle_frame wouldn't be called
> * for this slave anymore.
> @@ -1764,9 +1730,6 @@ static int __bond_release_one(struct net_device *bond_dev,
>
> bond->current_arp_slave = NULL;
>
> - /* release the slave from its bond */
> - bond_detach_slave(bond, slave);
> -
> if (!all && !bond->params.fail_over_mac) {
> if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) &&
> bond_has_slaves(bond))
>
Acked-by: Ding Tianhong@huawei.com
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Daniel Borkmann @ 2013-10-22 7:42 UTC (permalink / raw)
To: Ni, Xun
Cc: Daniel Wagner, Eric W. Biederman, pablo@netfilter.org,
netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
Tejun Heo, cgroups@vger.kernel.org
In-Reply-To: <91E2D863603AD4478F101CE81E76E45D01828D59@SHSMSX103.ccr.corp.intel.com>
On 10/22/2013 09:15 AM, Ni, Xun wrote:
> Hello, Daniel:
> can all your examples block early before doing network operations? What's the whole netfilter universe? Can you give us more clear examples?
As you can see from the code, the netfilter hooks are located
in NF_INET_LOCAL_OUT and NF_INET_POST_ROUTING.
> Thanks
> On 10/21/2013 05:09 PM, Daniel Wagner wrote:
>> On 10/19/2013 08:16 AM, Daniel Borkmann wrote:
>>> On 10/19/2013 01:21 AM, Eric W. Biederman wrote:
>>>
>>>> I am coming to this late. But two concrete suggestions.
>>>>
>>>> 1) process groups and sessions don't change as frequently as pids.
>>>>
>>>> 2) It is possible to put a set of processes in their own network
>>>> namespace and pipe just the packets you want those processes to
>>>> use into that network namespace. Using an ingress queueing filter
>>>> makes that process very efficient even if you have to filter by port.
>>>
>>> Actually in our case we're filtering outgoing traffic, based on which
>>> local socket that originated from; so you wouldn't need all of that
>>> construct. Also, you wouldn't even need to have an a-prio knowledge
>>> of the application internals regarding their use of particular use of
>>> ports or protocols. I don't think that such a setup will have the
>>> same efficiency, ease of use, and power to distinguish the
>>> application the traffic came from in such a lightweight, protocol independent and easy way.
>>
>> Sorry for beeing late as well (and also stupid question)
>>
>> Couldn't you use something from the LSM? I mean you allow the
>> application to create the socket etc and then block later the traffic
>> originated from that socket. Wouldn't it make more sense to block
>> early?
>
> I gave one simple example for blocking in the commit message, that's true, but it is not limited to that, meaning we can have much different scenarios/policies that netfilter allows us than just blocking, e.g. fine grained settings where applications are allowed to connect/send traffic to, application traffic marking/ conntracking, application-specific packet mangling, and so on, just think of the whole netfilter universe.
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xtables: lightweight process control group matching
From: Daniel Wagner @ 2013-10-22 7:45 UTC (permalink / raw)
To: Ni, Xun, Daniel Borkmann
Cc: Eric W. Biederman, pablo@netfilter.org,
netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
Tejun Heo, cgroups@vger.kernel.org
In-Reply-To: <91E2D863603AD4478F101CE81E76E45D01828D59@SHSMSX103.ccr.corp.intel.com>
Hi Xun,
On 10/22/2013 08:15 AM, Ni, Xun wrote:
> Hello, Daniel:
> can all your examples block early before doing network operations?
I was referring to Linux Security Module which allows
to define access policies for an application e.g. which ports are
allowed to be used.
If the goal is just to block those ports you don't have to go through
half of the networking stack to figure out via an iptable rules that
this access is not allowed.
> What's the whole netfilter universe? Can you give us more clear
> examples?
I am not sure if I understood your question correctly. In case you
are asking what netfilter is I would like pointing you to the
http://www.netfilter.org/ project page.
cheers,
daniel
^ permalink raw reply
* Re: [PATCH net-next v2] bonding: move bond-specific init after enslave happens
From: Jiri Pirko @ 2013-10-22 7:50 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, dingtianhong, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1382348910-32724-1-git-send-email-vfalico@redhat.com>
Mon, Oct 21, 2013 at 11:48:30AM CEST, vfalico@redhat.com wrote:
>As Jiri noted, currently we first do all bonding-specific initialization
>(specifically - bond_select_active_slave(bond)) before we actually attach
>the slave (so that it becomes visible through bond_for_each_slave() and
>friends). This might result in bond_select_active_slave() not seeing the
>first/new slave and, thus, not actually selecting an active slave.
>
>Fix this by moving all the bond-related init part after we've actually
>completely initialized and linked (via bond_master_upper_dev_link()) the
>new slave.
>
>Also, remove the bond_(de/a)ttach_slave(), it's useless to have functions
>to ++/-- one int.
>
>After this we have all the initialization of the new slave *before*
>linking, and all the stuff that needs to be done on bonding *after* it. It
>has also a bonus effect - we can remove the locking on the new slave init
>completely, and only use it for bond_select_active_slave().
>
>Reported-by: Jiri Pirko <jiri@resnulli.us>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Reviewed-by: Jiri Pirko <jiri@resnulli.us>
^ permalink raw reply
* [PATCH RESENT net-next] net: remove function sk_reset_txq()
From: ZHAO Gang @ 2013-10-22 8:23 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
What sk_reset_txq() does is just calls function sk_tx_queue_reset(),
and sk_reset_txq() is used only in sock.h, by dst_negative_advice().
Let dst_negative_advice() calls sk_tx_queue_reset() directly so we
can remove unneeded sk_reset_txq().
Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>
---
Hope this time I don't mess it up. Sorry for the inconvenience.
---
include/net/sock.h | 4 +---
net/core/sock.c | 6 ------
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 86bb066..c93542f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1746,8 +1746,6 @@ sk_dst_get(struct sock *sk)
return dst;
}
-void sk_reset_txq(struct sock *sk);
-
static inline void dst_negative_advice(struct sock *sk)
{
struct dst_entry *ndst, *dst = __sk_dst_get(sk);
@@ -1757,7 +1755,7 @@ static inline void dst_negative_advice(struct sock *sk)
if (ndst != dst) {
rcu_assign_pointer(sk->sk_dst_cache, ndst);
- sk_reset_txq(sk);
+ sk_tx_queue_clear(sk);
}
}
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 440afdc..ab20ed9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -475,12 +475,6 @@ discard_and_relse:
}
EXPORT_SYMBOL(sk_receive_skb);
-void sk_reset_txq(struct sock *sk)
-{
- sk_tx_queue_clear(sk);
-}
-EXPORT_SYMBOL(sk_reset_txq);
-
struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie)
{
struct dst_entry *dst = __sk_dst_get(sk);
--
1.8.3.1
^ permalink raw reply related
* Re: [virtio-net] BUG: sleeping function called from invalid context at kernel/mutex.c:616
From: Jason Wang @ 2013-10-22 8:35 UTC (permalink / raw)
To: Fengguang Wu; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20131020023418.GA6737@localhost>
[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]
On 10/20/2013 10:34 AM, Fengguang Wu wrote:
> Greetings,
>
> I got the below dmesg and the first bad commit is
>
> commit 3ab098df35f8b98b6553edc2e40234af512ba877
> Author: Jason Wang <jasowang@redhat.com>
> Date: Tue Oct 15 11:18:58 2013 +0800
>
> virtio-net: don't respond to cpu hotplug notifier if we're not ready
>
> We're trying to re-configure the affinity unconditionally in cpu hotplug
> callback. This may lead the issue during resuming from s3/s4 since
>
> - virt queues haven't been allocated at that time.
> - it's unnecessary since thaw method will re-configure the affinity.
>
> Fix this issue by checking the config_enable and do nothing is we're not ready.
>
> The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
> (virtio-net: reset virtqueue affinity when doing cpu hotplug).
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> [ 622.944441] CPU0 attaching NULL sched-domain.
> [ 622.944446] CPU1 attaching NULL sched-domain.
> [ 622.944485] CPU0 attaching NULL sched-domain.
> [ 622.950795] BUG: sleeping function called from invalid context at kernel/mutex.c:616
> [ 622.950796] in_atomic(): 1, irqs_disabled(): 1, pid: 10, name: migration/1
> [ 622.950796] no locks held by migration/1/10.
> [ 622.950798] CPU: 1 PID: 10 Comm: migration/1 Not tainted 3.12.0-rc5-wl-01249-gb91e82d #317
> [ 622.950799] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 622.950802] 0000000000000000 ffff88001d42dba0 ffffffff81a32f22 ffff88001bfb9c70
> [ 622.950803] ffff88001d42dbb0 ffffffff810edb02 ffff88001d42dc38 ffffffff81a396ed
> [ 622.950805] 0000000000000046 ffff88001d42dbe8 ffffffff810e861d 0000000000000000
> [ 622.950805] Call Trace:
> [ 622.950810] [<ffffffff81a32f22>] dump_stack+0x54/0x74
> [ 622.950815] [<ffffffff810edb02>] __might_sleep+0x112/0x114
> [ 622.950817] [<ffffffff81a396ed>] mutex_lock_nested+0x3c/0x3c6
> [ 622.950818] [<ffffffff810e861d>] ? up+0x39/0x3e
> [ 622.950821] [<ffffffff8153ea7c>] ? acpi_os_signal_semaphore+0x21/0x2d
> [ 622.950824] [<ffffffff81565ed1>] ? acpi_ut_release_mutex+0x5e/0x62
> [ 622.950828] [<ffffffff816d04ec>] virtnet_cpu_callback+0x33/0x87
> [ 622.950830] [<ffffffff81a42576>] notifier_call_chain+0x3c/0x5e
> [ 622.950832] [<ffffffff810e86a8>] __raw_notifier_call_chain+0xe/0x10
> [ 622.950835] [<ffffffff810c5556>] __cpu_notify+0x20/0x37
> [ 622.950836] [<ffffffff810c5580>] cpu_notify+0x13/0x15
> [ 622.950838] [<ffffffff81a237cd>] take_cpu_down+0x27/0x3a
> [ 622.950841] [<ffffffff81136289>] stop_machine_cpu_stop+0x93/0xf1
> [ 622.950842] [<ffffffff81136167>] cpu_stopper_thread+0xa0/0x12f
> [ 622.950844] [<ffffffff811361f6>] ? cpu_stopper_thread+0x12f/0x12f
> [ 622.950847] [<ffffffff81119710>] ? lock_release_holdtime.part.7+0xa3/0xa8
> [ 622.950848] [<ffffffff81135e4b>] ? cpu_stop_should_run+0x3f/0x47
> [ 622.950850] [<ffffffff810ea9b0>] smpboot_thread_fn+0x1c5/0x1e3
> [ 622.950852] [<ffffffff810ea7eb>] ? lg_global_unlock+0x67/0x67
> [ 622.950854] [<ffffffff810e36b7>] kthread+0xd8/0xe0
> [ 622.950857] [<ffffffff81a3bfad>] ? wait_for_common+0x12f/0x164
> [ 622.950859] [<ffffffff810e35df>] ? kthread_create_on_node+0x124/0x124
> [ 622.950861] [<ffffffff81a45ffc>] ret_from_fork+0x7c/0xb0
> [ 622.950862] [<ffffffff810e35df>] ? kthread_create_on_node+0x124/0x124
> [ 622.950876] smpboot: CPU 1 is now offline
> [ 623.194556] SMP alternatives: lockdep: fixing up alternatives
> [ 623.194559] smpboot: Booting Node 0 Processor 1 APIC 0x1
Thanks for the testing Fengguang, could you please try the attached
patch to see if it works?
[-- Attachment #2: 0001-virtio-net-fix.patch --]
[-- Type: text/x-patch, Size: 1464 bytes --]
>From 01e6c3f71c202aa02e4feda169e7cc9fb24193f5 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 21 Oct 2013 20:39:09 +0800
Subject: [PATCH] virtio-net: fix
---
drivers/net/virtio_net.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9fbdfcd..bbc9cb8 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1118,11 +1118,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
{
struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
- mutex_lock(&vi->config_lock);
-
- if (!vi->config_enable)
- goto done;
-
switch(action & ~CPU_TASKS_FROZEN) {
case CPU_ONLINE:
case CPU_DOWN_FAILED:
@@ -1136,8 +1131,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
break;
}
-done:
- mutex_unlock(&vi->config_lock);
return NOTIFY_OK;
}
@@ -1699,6 +1692,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
struct virtnet_info *vi = vdev->priv;
int i;
+ unregister_hotcpu_notifier(&vi->nb);
+
/* Prevent config work handler from accessing the device */
mutex_lock(&vi->config_lock);
vi->config_enable = false;
@@ -1747,6 +1742,10 @@ static int virtnet_restore(struct virtio_device *vdev)
virtnet_set_queues(vi, vi->curr_queue_pairs);
rtnl_unlock();
+ err = register_hotcpu_notifier(&vi->nb);
+ if (err)
+ return err;
+
return 0;
}
#endif
--
1.8.1.2
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH RESEND] packet: Deliver VLAN TPID to userspace
From: Atzm Watanabe @ 2013-10-22 8:39 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Ben Hutchings
After the 802.1AD support, userspace packet receivers
(packet dumper, software switch, and the like) need how to know
VLAN TPID in order to reassemble original tagged frame.
Signed-off-by: Atzm Watanabe <atzm@stratosphere.co.jp>
---
struct tpacket_hdr_variant1 looks like that is allowed to grow,
as the length combined with struct tpacket3_hdr is explicit at
run-time.
include/uapi/linux/if_packet.h | 5 +++--
net/packet/af_packet.c | 8 ++++++--
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
index dbf0666..6e36e0a 100644
--- a/include/uapi/linux/if_packet.h
+++ b/include/uapi/linux/if_packet.h
@@ -83,7 +83,7 @@ struct tpacket_auxdata {
__u16 tp_mac;
__u16 tp_net;
__u16 tp_vlan_tci;
- __u16 tp_padding;
+ __u16 tp_vlan_tpid;
};
/* Rx ring - header status */
@@ -132,12 +132,13 @@ struct tpacket2_hdr {
__u32 tp_sec;
__u32 tp_nsec;
__u16 tp_vlan_tci;
- __u16 tp_padding;
+ __u16 tp_vlan_tpid;
};
struct tpacket_hdr_variant1 {
__u32 tp_rxhash;
__u32 tp_vlan_tci;
+ __u32 tp_vlan_tpid;
};
struct tpacket3_hdr {
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..fbcc882 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -895,9 +895,11 @@ static void prb_fill_vlan_info(struct tpacket_kbdq_core *pkc,
{
if (vlan_tx_tag_present(pkc->skb)) {
ppd->hv1.tp_vlan_tci = vlan_tx_tag_get(pkc->skb);
+ ppd->hv1.tp_vlan_tpid = (__force __u32)ntohs(pkc->skb->vlan_proto);
ppd->tp_status = TP_STATUS_VLAN_VALID;
} else {
ppd->hv1.tp_vlan_tci = 0;
+ ppd->hv1.tp_vlan_tpid = 0;
ppd->tp_status = TP_STATUS_AVAILABLE;
}
}
@@ -1836,11 +1838,12 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
h.h2->tp_nsec = ts.tv_nsec;
if (vlan_tx_tag_present(skb)) {
h.h2->tp_vlan_tci = vlan_tx_tag_get(skb);
+ h.h2->tp_vlan_tpid = ntohs(skb->vlan_proto);
status |= TP_STATUS_VLAN_VALID;
} else {
h.h2->tp_vlan_tci = 0;
+ h.h2->tp_vlan_tpid = 0;
}
- h.h2->tp_padding = 0;
hdrlen = sizeof(*h.h2);
break;
case TPACKET_V3:
@@ -2788,11 +2791,12 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
aux.tp_net = skb_network_offset(skb);
if (vlan_tx_tag_present(skb)) {
aux.tp_vlan_tci = vlan_tx_tag_get(skb);
+ aux.tp_vlan_tpid = ntohs(skb->vlan_proto);
aux.tp_status |= TP_STATUS_VLAN_VALID;
} else {
aux.tp_vlan_tci = 0;
+ aux.tp_vlan_tpid = 0;
}
- aux.tp_padding = 0;
put_cmsg(msg, SOL_PACKET, PACKET_AUXDATA, sizeof(aux), &aux);
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH net] netpoll: fix rx_hook() interface by passing the skb
From: Antonio Quartulli @ 2013-10-22 8:48 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Antonio Quartulli
In-Reply-To: <20131022.025038.1046903740187748879.davem@davemloft.net>
Right now skb->data is passed to rx_hook() even if the skb
has not been linearised and without giving rx_hook() a way
to linearise it.
Change the rx_hook() interface and make it accept the skb
as argument. In this way users implementing rx_hook() can
perform all the needed operations to properly (and safely)
access the skb data.
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
include/linux/netpoll.h | 2 +-
net/core/netpoll.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index f3c7c24..5352160 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -24,7 +24,7 @@ struct netpoll {
struct net_device *dev;
char dev_name[IFNAMSIZ];
const char *name;
- void (*rx_hook)(struct netpoll *, int, char *, int);
+ void (*rx_hook)(struct netpoll *np, struct sk_buff *skb, int offset);
union inet_addr local_ip, remote_ip;
bool ipv6;
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index fc75c9e..b415437 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -834,9 +834,8 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
if (np->local_port && np->local_port != ntohs(uh->dest))
continue;
- np->rx_hook(np, ntohs(uh->source),
- (char *)(uh+1),
- ulen - sizeof(struct udphdr));
+ np->rx_hook(np, skb,
+ (unsigned char *)(uh + 1) - skb->data);
hits++;
}
} else {
@@ -872,9 +871,8 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
if (np->local_port && np->local_port != ntohs(uh->dest))
continue;
- np->rx_hook(np, ntohs(uh->source),
- (char *)(uh+1),
- ulen - sizeof(struct udphdr));
+ np->rx_hook(np, skb,
+ (unsigned char *)(uh + 1) - skb->data);
hits++;
}
#endif
--
1.8.4
^ permalink raw reply related
* RE: [PATCH net] netpoll: fix rx_hook() interface by passing the skb
From: David Laight @ 2013-10-22 9:09 UTC (permalink / raw)
To: Antonio Quartulli, David S. Miller; +Cc: netdev
In-Reply-To: <1382431715-3128-1-git-send-email-antonio@meshcoding.com>
> Subject: [PATCH net] netpoll: fix rx_hook() interface by passing the skb
>
> Right now skb->data is passed to rx_hook() even if the skb
> has not been linearised and without giving rx_hook() a way
> to linearise it.
>
> Change the rx_hook() interface and make it accept the skb
> as argument. In this way users implementing rx_hook() can
> perform all the needed operations to properly (and safely)
> access the skb data.
...
> - void (*rx_hook)(struct netpoll *, int, char *, int);
> + void (*rx_hook)(struct netpoll *np, struct sk_buff *skb, int offset);
You can't do that change without changing the way that hooks are registered
so that any existing modules will fail to register their hooks.
David
^ 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