* Re: [patch] rtnetlink: remove an unneeded test
From: Eric W. Biederman @ 2013-08-14 19:35 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, John Fastabend, Jiri Pirko, Vlad Yasevich,
netdev, kernel-janitors
In-Reply-To: <20130814093542.GC521@elgon.mountain>
Dan Carpenter <dan.carpenter@oracle.com> writes:
> We know that "dev" is a valid pointer at this point, so we can remove
> the test and clean up a little.
Quite right.
Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 618dbe4..e7e058e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1867,10 +1867,10 @@ replay:
> else
> err = register_netdevice(dev);
>
> - if (err < 0 && !IS_ERR(dev))
> + if (err < 0) {
> free_netdev(dev);
> - if (err < 0)
> goto out;
> + }
>
> err = rtnl_configure_link(dev, ifm);
> if (err < 0)
^ permalink raw reply
* TCP Connection teardown seems to violate TCP specification
From: Damian Lukowski @ 2013-08-14 19:21 UTC (permalink / raw)
To: netdev
Hi,
the TCP specification states that an endpoint has to accept
packets from the other side even after it has half-closed the
connection locally. This does not seem to be the case
under Linux?
For example, when I wget -O /dev/null https://www.verisign.com/
--max-redir 0, the local TCP stack sends an FIN,ACK close to the end.
However, the webserver has more data to send (Encrypted Alert).
Instead of ACKing the packet, the local side sends a RST.
This seems wrong to me.
Regards
Damian
^ permalink raw reply
* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Michael S. Tsirkin @ 2013-08-14 19:24 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1376499820-27764-1-git-send-email-vyasevic@redhat.com>
On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work. This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.
Just to clarify - is there some userspace that actually
triggers this?
> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled. Treat any attempt to enable offloads as an error in
> this case.
> We also need to update the TUN_FEATURES mask to include all checksum
> options as the underlying device may have something other then
> HW_CSUM set.
>
> Change since v1:
> - Removed the call to update offloads when IFF_VNET_HDR is turned off.
> - Changed the macvtap version of TUN_OFFLOADS to include all checksum
> offloads since the physical nic may have them set.
> - Treat enabling of offloads without vnet_hdr support as error.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> drivers/net/macvtap.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..3acfc37 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
>
> static const struct proto_ops macvtap_socket_ops;
>
> -#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> +#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> NETIF_F_TSO6 | NETIF_F_UFO)
> #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
> /*
> @@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> if (!vlan)
> return -ENOLINK;
>
> + /* If the user is trying to set offloads while IFF_VNET_HDR is
> + * off, report it as an error.
> + */
> + if (!(q->flags & IFF_VNET_HDR) && arg)
> + return -EINVAL;
> +
> features = vlan->dev->features;
>
> if (arg & TUN_F_CSUM) {
> @@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> TUN_F_TSO_ECN | TUN_F_UFO))
> return -EINVAL;
>
> - /* TODO: only accept frames with the features that
> - got enabled for forwarded frames */
> - if (!(q->flags & IFF_VNET_HDR))
> - return -EINVAL;
> rtnl_lock();
> ret = set_offload(q, arg);
> rtnl_unlock();
> --
> 1.8.1.4
^ permalink raw reply
* Re: [PATCH v3 00/11] Add namespace support for syslog
From: Eric W. Biederman @ 2013-08-14 19:21 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
libo.chen-hv44wF8Li93QT0dZR+AlfA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
guz.fnst-BthXqXjhjHXQFUHtdCDX3A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b
In-Reply-To: <20130814153017.GA18403-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
"Serge E. Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> writes:
> Quoting Rui Xiang (rui.xiang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org):
>> On 2013/8/8 9:37, Gao feng wrote:
>> > On 08/07/2013 03:55 PM, Eric W. Biederman wrote:
>> >>
>> >> Since this still has not been addressed. I am going to repeat Andrews
>> >> objection again.
>> >>
>> >> Isn't there a better way to get iptables information out than to use
>> >> syslog. I did not have time to follow up on that but it did appear that
>> >> someone did have a better way to get the information out.
>> >>
>> >> Essentially the argument against this goes. The kernel logging facility
>> >> is really not a particularly good tool to be using for anything other
>> >> than kernel debugging information, and there appear to be no substantial
>> >> uses for a separate syslog that should not be done in other ways.
>> >
>> > containerizing syslog is not only for iptables, it also isolates the /dev/kmsg,
>> > /proc/kmsg, syslog(2)... user space tools in container may use this interface
>> > to read/generate syslog.
>> >
>> > But I don't know how important/urgent this containerizing syslog work is,
>> > Rui Xiang, can you find an important/popular user space tool which uses this
>> > interfaces to generate kernel syslog?
>> >
>>
>> There are some other cases. Some warnings (bad mount options for tmpfs,
>> bad uid owner for many of them, etc) emerged in the container should
>> be exported to the container. Some belong on the host - if they show
>> a corrupt superblock which may indicate an attempt by the container
>> to crash the kernel. Like these, Kernel will print out warnings when
>> userspace in container uses a deprecated something or other, and these
>> logs should be invisible and specific for current container.
>>
>> I can't say this work is terribly compelling and important, but the
>> impact may be obvious, IMO.
>
> Aug 9 21:49:13 sergeh1 kernel: [4644829.672768] init: Failed to spawn network-interface (veth8Ehlvj) post-stop process: unable to change root directory: No such file ricr:aeohgrticr cfe rty444984 n:aetswnw-ta(ht -rrsultheoit: hlrro<4865i:i sntkta(ht ttpe btheoit: hlrrob r6ezt)nrgoadgte644915 c0pt(tyg ti wd a
> Aug 9 21:49:13 sergeh1 kernel: X3f d-6:uigitra ora
> Aug 9 22:19:54 sergeh1 kernel: 6[642.175 X3f d-6:mutdflsse ihodrddt oe==99 rfl=lccnanrdfutwt-etn"nm=/a/ah/x/lu-ui/m.AExdu"pi=91 om=mut sye"x3name="/devlo0" lg=r"ol pmc r=3an19pfel-nireu-tntgne/rc/cldudmHEqu d97o=otfy=x"ra=d/o/fg""8b:o vhc)nrgibdte646013 veeMzWe oso d<[4715]xr r1eMc egset
> Aug
That is certainly a mess. Now I don't believe we allow processes in a
user namespace to write to the kernels log (certainly we shouldn't be)
so part of that is not a problem.
What is interleaving messages into syslog?
And to be clear my only perspective is that we need to make certain we
have this thought out.
Eric
^ permalink raw reply
* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Michael S. Tsirkin @ 2013-08-14 19:23 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <520BD719.6010601@redhat.com>
On Wed, Aug 14, 2013 at 03:14:33PM -0400, Vlad Yasevich wrote:
> On 08/14/2013 01:50 PM, Michael S. Tsirkin wrote:
> >On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
> >>When the user turns off IFF_VNET_HDR flag, attempts to change
> >>offload features via TUNSETOFFLOAD do not work. This could cause
> >>GSO packets to be delivered to the user when the user is
> >>not prepared to handle them.
> >>
> >>To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> >>disabled. Treat any attempt to enable offloads as an error in
> >>this case.
> >>We also need to update the TUN_FEATURES mask to include all checksum
> >>options as the underlying device may have something other then
> >>HW_CSUM set.
> >
> >This last looks like a completely unrelated change,
> >does it not?
> >Would be nice to have it in a separate patchset with some
> >examples of broken configurations.
>
> sure. can do.
>
> >
> >>
> >>Change since v1:
> >> - Removed the call to update offloads when IFF_VNET_HDR is turned off.
> >> - Changed the macvtap version of TUN_OFFLOADS to include all checksum
> >> offloads since the physical nic may have them set.
> >> - Treat enabling of offloads without vnet_hdr support as error.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>---
> >> drivers/net/macvtap.c | 12 +++++++-----
> >> 1 file changed, 7 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >>index a98fb0e..3acfc37 100644
> >>--- a/drivers/net/macvtap.c
> >>+++ b/drivers/net/macvtap.c
> >>@@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
> >>
> >> static const struct proto_ops macvtap_socket_ops;
> >>
> >>-#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> >>+#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> >> NETIF_F_TSO6 | NETIF_F_UFO)
> >> #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
> >> /*
> >>@@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> >> if (!vlan)
> >> return -ENOLINK;
> >>
> >>+ /* If the user is trying to set offloads while IFF_VNET_HDR is
> >>+ * off, report it as an error.
> >>+ */
> >>+ if (!(q->flags & IFF_VNET_HDR) && arg)
> >>+ return -EINVAL;
> >>+
> >
> >This function has a single caller so it should matter,
> >but I'm just curious why are you moving the test here from
> >macvtap_ioctl?
> >
>
> I thought the same thing. I can certainly move it back. Will make
> the patch smaller.
>
> >> features = vlan->dev->features;
> >>
> >> if (arg & TUN_F_CSUM) {
> >>@@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> >> TUN_F_TSO_ECN | TUN_F_UFO))
> >> return -EINVAL;
> >>
> >>- /* TODO: only accept frames with the features that
> >>- got enabled for forwarded frames */
> >
> >Why do you drop this btw? you disagree we should do this
> >eventually? Maybe a separate patch too.
>
> I thought that this comment related to the check for VNET_HDR.
> I am having a hard time understanding the meaning behind this comment.
> Is the meaning that if GSO is disabled, we shouldn't accept GSO? This
> kind the reverse of what we've been doing.
I see, you mean 3e4f8b787370978733ca6cae452720a4f0c296b8
actually addressed this comment?
I think you are right, and we can drop it, though maybe cleaner
to do it by a separate patch.
> Or is more along the lines of dropping dropping GSO_GRE for now since
> we don't support that offload function yet?
>
> -vlad
Confused. What don't we support?
> >
> >>- if (!(q->flags & IFF_VNET_HDR))
> >>- return -EINVAL;
> >> rtnl_lock();
> >> ret = set_offload(q, arg);
> >> rtnl_unlock();
> >>--
> >>1.8.1.4
^ permalink raw reply
* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Vlad Yasevich @ 2013-08-14 19:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20130814175014.GB17580@redhat.com>
On 08/14/2013 01:50 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
>> When the user turns off IFF_VNET_HDR flag, attempts to change
>> offload features via TUNSETOFFLOAD do not work. This could cause
>> GSO packets to be delivered to the user when the user is
>> not prepared to handle them.
>>
>> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
>> disabled. Treat any attempt to enable offloads as an error in
>> this case.
>> We also need to update the TUN_FEATURES mask to include all checksum
>> options as the underlying device may have something other then
>> HW_CSUM set.
>
> This last looks like a completely unrelated change,
> does it not?
> Would be nice to have it in a separate patchset with some
> examples of broken configurations.
sure. can do.
>
>>
>> Change since v1:
>> - Removed the call to update offloads when IFF_VNET_HDR is turned off.
>> - Changed the macvtap version of TUN_OFFLOADS to include all checksum
>> offloads since the physical nic may have them set.
>> - Treat enabling of offloads without vnet_hdr support as error.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> drivers/net/macvtap.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> index a98fb0e..3acfc37 100644
>> --- a/drivers/net/macvtap.c
>> +++ b/drivers/net/macvtap.c
>> @@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
>>
>> static const struct proto_ops macvtap_socket_ops;
>>
>> -#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
>> +#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
>> NETIF_F_TSO6 | NETIF_F_UFO)
>> #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
>> /*
>> @@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>> if (!vlan)
>> return -ENOLINK;
>>
>> + /* If the user is trying to set offloads while IFF_VNET_HDR is
>> + * off, report it as an error.
>> + */
>> + if (!(q->flags & IFF_VNET_HDR) && arg)
>> + return -EINVAL;
>> +
>
> This function has a single caller so it should matter,
> but I'm just curious why are you moving the test here from
> macvtap_ioctl?
>
I thought the same thing. I can certainly move it back. Will make the
patch smaller.
>> features = vlan->dev->features;
>>
>> if (arg & TUN_F_CSUM) {
>> @@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>> TUN_F_TSO_ECN | TUN_F_UFO))
>> return -EINVAL;
>>
>> - /* TODO: only accept frames with the features that
>> - got enabled for forwarded frames */
>
> Why do you drop this btw? you disagree we should do this
> eventually? Maybe a separate patch too.
I thought that this comment related to the check for VNET_HDR.
I am having a hard time understanding the meaning behind this comment.
Is the meaning that if GSO is disabled, we shouldn't accept GSO? This
kind the reverse of what we've been doing.
Or is more along the lines of dropping dropping GSO_GRE for now since
we don't support that offload function yet?
-vlad
>
>> - if (!(q->flags & IFF_VNET_HDR))
>> - return -EINVAL;
>> rtnl_lock();
>> ret = set_offload(q, arg);
>> rtnl_unlock();
>> --
>> 1.8.1.4
^ permalink raw reply
* Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called
From: Sergei Shtylyov @ 2013-08-14 19:04 UTC (permalink / raw)
To: Fan Du
Cc: tglx, Steffen Klassert, David Miller, Herbert Xu, Daniel Borkmann,
LKML, netdev
In-Reply-To: <520B4552.2000607@windriver.com>
Hello.
On 08/14/2013 12:52 PM, Fan Du wrote:
> From e3929d4fdfad5b40fd8cad0e217597670d1aef54 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Wed, 14 Aug 2013 16:39:23 +0800
> Subject: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called
This header is not needed.
> When clock_was_set is called in case of system wall time change
> or host resume from suspend state, use this notifier for places
> where interested in this action, e.g Ipsec SA lifetime management.
> Signed-off-by: Fan Du <fan.du@windriver.com>
> v3:
> -Beautify notifier with register/unregister API exported for other subsystem.
[...]
> diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
> index d19a5c2..f0404e4 100644
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -461,4 +461,7 @@ extern u64 ktime_divns(const ktime_t kt, s64 div);
> /* Show pending timers: */
> extern void sysrq_timer_list_show(void);
>
> +extern int register_clock_change_notifier(struct notifier_block *nb);
> +extern int unregister_clock_change_notifier(struct notifier_block *nb);
> +
> #endif
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 383319b..c6e6405 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -755,6 +755,24 @@ static inline void retrigger_next_event(void *arg) { }
>
> #endif /* CONFIG_HIGH_RES_TIMERS */
>
> +static ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
> +static int call_clock_change_notifiers(void)
> +{
> + return atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);
> +}
> +
> +int register_clock_change_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_register(&clock_change_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_clock_change_notifier);
> +
> +int unregister_clock_change_notifier(struct notifier_block *nb)
> +{
> + return atomic_notifier_chain_unregister(&clock_change_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_clock_change_notifier);
> +
> /*
> * Clock realtime was set
> *
> @@ -773,6 +791,7 @@ void clock_was_set(void)
> on_each_cpu(retrigger_next_event, NULL, 1);
> #endif
> timerfd_clock_was_set();
> + call_clock_change_notifiers();
> }
Your patch seems whitepsace damaged, i.e. a space is added before each
line starting with space.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH RESEND 0/8] use platform_{get,set}_drvdata()
From: Sergei Shtylyov @ 2013-08-14 18:56 UTC (permalink / raw)
To: Libo Chen
Cc: David Miller, netdev, LKML, Li Zefan, leoli, linuxppc-dev,
pantelis.antoniou, vbordug, Greg KH, jg1.han
In-Reply-To: <520AFB3F.2090006@huawei.com>
On 08/14/2013 07:36 AM, Libo Chen wrote:
> We can use the wrapper functions platform_{get,set}_drvdata() instead of
> dev_{get,set}_drvdata() with &pdev->dev, it is convenient for user.
>
> Also, unnecessary dev_set_drvdata() is removed, because the driver core
> clears the driver data to NULL after device_release or on probe failure.
Saying "also" in the changelog is usually a good sign that the patch
should be split.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH RESEND 1/8] net: fsl_pq_mdio: use platform_{get,set}_drvdata()
From: Sergei Shtylyov @ 2013-08-14 18:54 UTC (permalink / raw)
To: Libo Chen; +Cc: David Miller, netdev, LKML, Li Zefan
In-Reply-To: <520AFBEE.7030903@huawei.com>
Hello.
On 08/14/2013 07:39 AM, Libo Chen wrote:
> We can use the wrapper functions platform_{get,set}_drvdata() instead of
> dev_{get,set}_drvdata() with &pdev->dev, it is convenient for user.
> Also, unnecessary dev_set_drvdata() is removed, because the driver core
> clears the driver data to NULL after device_release or on probe failure.
You misunderstood. That's a material for 2 separate patch sets (or maybe
single patches, since multiple files can be collapsed in a single patch).
> Signed-off-by: Libo Chen <libo.chen@huawei.com>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] skge: dma_sync the whole receive buffer
From: poma @ 2013-08-14 18:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20130814092022.494caf2b@nehalam.linuxnetplumber.net>
On 14.08.2013 18:20, Stephen Hemminger wrote:
> On Wed, 14 Aug 2013 12:20:03 +0200
> poma <pomidorabelisima@gmail.com> wrote:
>
>> On 14.08.2013 03:00, Stephen Hemminger wrote:
>>> On Tue, 13 Aug 2013 15:09:55 -0700 (PDT)
>>> David Miller <davem@davemloft.net> wrote:
>>>
>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>> Date: Sat, 10 Aug 2013 15:02:07 -0700
>>>>
>>>>> The DMA sync should sync the whole receive buffer, not just
>>>>> part of it. Fixes log messages dma_sync_check.
>>>>>
>>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>>
>>>> Applied, but I really suspect that your "check DMA mapping errors"
>>>> patch has added a serious regression. A regression much worse than
>>>> the bug you were trying to fix with that change.
>>>
>>> Argh. The problem is deeper than that. Device got broken somewhere between
>>> 3.2 and 3.4. My old Dlink card works on 3.2 but gets DMA errors on 3.4.
>>> The config's are different though so checking that as well.
>>>
>>
>> Can I help you with debugging?
>> DGE-530T is rather solid device.
>
> Don't think it is a hardware problem.
> The failure is when the board access the Receive ring PCI memory area.
> This region is allocated with pci_alloc_consistent and therefore should
> be available. Two possible issues are driver math issues, or hardware
> problems with where the region is located. Some of these cards don't
> really have full 64 bit PCI support.
>
> My board is:
> 05:01.0 Ethernet controller: D-Link System Inc Gigabit Ethernet Adapter (rev 11)
> Subsystem: D-Link System Inc DGE-530T Gigabit Ethernet Adapter
> Flags: bus master, 66MHz, medium devsel, latency 32, IRQ 18
> Memory at f7d20000 (32-bit, non-prefetchable) [size=16K]
> I/O ports at c000 [size=256]
> Expansion ROM at f7d00000 [disabled] [size=128K]
> Capabilities: [48] Power Management version 2
> Capabilities: [50] Vital Product Data
> Kernel driver in use: skge
>
>
> What is your config?
>
01:09.0 Ethernet controller: D-Link System Inc Gigabit Ethernet Adapter
(rev 11)
Subsystem: D-Link System Inc DGE-530T Gigabit Ethernet Adapter
Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 19
Memory at fbffc000 (32-bit, non-prefetchable) [size=16K]
I/O ports at b400 [size=256]
[virtual] Expansion ROM at ec000000 [disabled] [size=128K]
Capabilities: [48] Power Management version 2
Capabilities: [50] Vital Product Data
Kernel driver in use: skge
poma
^ permalink raw reply
* cpsw fails to come up once in a while
From: Sebastian Andrzej Siewior @ 2013-08-14 17:54 UTC (permalink / raw)
To: Mugunthan V N; +Cc: netdev
Hi,
So I have this test where my am335x-evm boots over NFS and once it
completes it does it again. It works most of the time :) However once it
like 100 attempts it does not. After the reset u-boot complains about
ethernet timeout and linux has also no luck:
| U-Boot 2013.04-00274-ga71d45d (May 27 2013 - 12:41:57)
|
|…
| Net: Phy not found
| PHY reset timed out
| cpsw, usb_ether
| Hit any key to stop autoboot: 1 ^H^H^H 0
| link up on port 0, speed 1000, full duplex
| Using cpsw device
| TFTP from server 172.123.10.10; our IP address is 172.123.10.3
| Filename 'am335x/zImage'.
| Load address: 0x80200000
| Loading: *^H#################################################################
| #################################################################
| #######################################
| 509.8 KiB/s
So the PHY was not found but somehow it managed to fetch the kernel over
network and boot it. The linux driver did not want to continue:
| omap_i2c 4802a000.i2c: bus 1 rev0.11 at 100 kHz
| davinci_mdio 4a101000.mdio: davinci mdio revision 1.6
| davinci_mdio 4a101000.mdio: no live phy, scanning all
| davinci_mdio: probe of 4a101000.mdio failed with error -5
| Random MACID = 16:74:44:51:f1:0f
| gpio-keys volume_keys.6: Unable to claim irq 0; error -22
| gpio-keys: probe of volume_keys.6 failed with error -22
| omap_rtc 44e3e000.rtc: setting system clock to 2000-01-01 03:22:52 UTC (946696972)
| net eth0: initializing cpsw version 1.12 (0)
| libphy: PHY 4a101000.mdio:00 not found
| net eth0: phy 4a101000.mdio:00 not found on slave 0
| libphy: PHY 4a101000.mdio:01 not found
| net eth0: phy 4a101000.mdio:01 not found on slave 1
| Waiting up to 110 more seconds for network.
so the driver also failed to find a PHY and gave up.
Is this something known?
Sebastian
^ permalink raw reply
* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Michael S. Tsirkin @ 2013-08-14 17:50 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1376499820-27764-1-git-send-email-vyasevic@redhat.com>
On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work. This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.
>
> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled. Treat any attempt to enable offloads as an error in
> this case.
> We also need to update the TUN_FEATURES mask to include all checksum
> options as the underlying device may have something other then
> HW_CSUM set.
This last looks like a completely unrelated change,
does it not?
Would be nice to have it in a separate patchset with some
examples of broken configurations.
>
> Change since v1:
> - Removed the call to update offloads when IFF_VNET_HDR is turned off.
> - Changed the macvtap version of TUN_OFFLOADS to include all checksum
> offloads since the physical nic may have them set.
> - Treat enabling of offloads without vnet_hdr support as error.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> drivers/net/macvtap.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..3acfc37 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
>
> static const struct proto_ops macvtap_socket_ops;
>
> -#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> +#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> NETIF_F_TSO6 | NETIF_F_UFO)
> #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
> /*
> @@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> if (!vlan)
> return -ENOLINK;
>
> + /* If the user is trying to set offloads while IFF_VNET_HDR is
> + * off, report it as an error.
> + */
> + if (!(q->flags & IFF_VNET_HDR) && arg)
> + return -EINVAL;
> +
This function has a single caller so it should matter,
but I'm just curious why are you moving the test here from
macvtap_ioctl?
> features = vlan->dev->features;
>
> if (arg & TUN_F_CSUM) {
> @@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> TUN_F_TSO_ECN | TUN_F_UFO))
> return -EINVAL;
>
> - /* TODO: only accept frames with the features that
> - got enabled for forwarded frames */
Why do you drop this btw? you disagree we should do this
eventually? Maybe a separate patch too.
> - if (!(q->flags & IFF_VNET_HDR))
> - return -EINVAL;
> rtnl_lock();
> ret = set_offload(q, arg);
> rtnl_unlock();
> --
> 1.8.1.4
^ permalink raw reply
* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Michael S. Tsirkin @ 2013-08-14 17:43 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <520BAA86.1060204@redhat.com>
On Wed, Aug 14, 2013 at 12:04:22PM -0400, Vlad Yasevich wrote:
> On 08/14/2013 10:07 AM, Vlad Yasevich wrote:
> >On 08/14/2013 08:10 AM, Michael S. Tsirkin wrote:
> >>On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
> >>>When the user turns off IFF_VNET_HDR flag, attempts to change
> >>>offload features via TUNSETOFFLOAD do not work. This could cause
> >>>GSO packets to be delivered to the user when the user is
> >>>not prepared to handle them.
> >>>
> >>>To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> >>>disabled and make sure to turn off all offloads in this case.
> >>>Also, when IFF_VNET_HDR is disabled, run throught the offload change
> >>>as well to make sure that the functionality is not dependent on
> >>>the order of the ioclt() calls.
> >>>
> >>>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>
> >>Hmm, this seems to make it even messier.
> >>So
> >> set TUNSETOFFLOAD
> >> clear IFF_VNET_HDR
> >> set IFF_VNET_HDR
> >>
> >>offloads are now clear?
> >>
> >>I'm also not sure userspace that sets offloads but clears
> >>vnet hdr knows what it's doing.
> >>
> >>How about we fail attempts to clear vnet hdr
> >>unless offloads are disabled?
> >>
> >
> >OK. That makes it simpler...
>
> Spoke too soon.
>
> What happens is that libvirt is typically the one that turns off
> IFF_VNET_HDR, and qemu controls the offloads.
>
> So, if we forbid turning off vnet_hdr while offloads are set, that
> will break libvirt (this is because macvtap defaults to tap offloads
> being enabled).
>
> -vlad
Well what prevents libvirt from disabling offloads?
Anyway, another approach is to say that this is up to user -
if user disables hdr but not offloads, user will get
packets without a checksum.
> >
> >-vlad
> >
> >>
> >>
> >>
> >>>---
> >>> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
> >>> 1 file changed, 19 insertions(+), 8 deletions(-)
> >>>
> >>>diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >>>index a98fb0e..076b9e7 100644
> >>>--- a/drivers/net/macvtap.c
> >>>+++ b/drivers/net/macvtap.c
> >>>@@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q,
> >>>unsigned long arg)
> >>> struct macvlan_dev *vlan;
> >>> netdev_features_t features;
> >>> netdev_features_t feature_mask = 0;
> >>>+ netdev_features_t tap_mask = TUN_OFFLOADS;
> >>>
> >>> vlan = rtnl_dereference(q->vlan);
> >>> if (!vlan)
> >>>@@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue
> >>>*q, unsigned long arg)
> >>>
> >>> features = vlan->dev->features;
> >>>
> >>>- if (arg & TUN_F_CSUM) {
> >>>+ if (!(q->flags & IFF_VNET_HDR)) {
> >>>+ /* Turn off all checsum offloading also if user does
> >>>+ * not user vnet_hdr.
> >>
> >>Does not set?
> >>
> >>>+ */
> >>>+ tap_mask |= NETIF_F_ALL_CSUM;
> >>>+ } else if (arg & TUN_F_CSUM) {
> >>> feature_mask = NETIF_F_HW_CSUM;
> >>>
> >>> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
> >>>@@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q,
> >>>unsigned long arg)
> >>> /* tap_features are the same as features on tun/tap and
> >>> * reflect user expectations.
> >>> */
> >>>- vlan->tap_features = vlan->dev->features &
> >>>- (feature_mask | ~TUN_OFFLOADS);
> >>>+ vlan->tap_features = vlan->dev->features & (feature_mask |
> >>>~tap_mask);
> >>> vlan->set_features = features;
> >>> netdev_update_features(vlan->dev);
> >>>
> >>>@@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file,
> >>>unsigned int cmd,
> >>> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
> >>> (IFF_NO_PI | IFF_TAP))
> >>> ret = -EINVAL;
> >>>- else
> >>>+ else {
> >>>+ if ((q->flags ^ u) & IFF_VNET_HDR) {
> >>>+ /* vnet_hdr support impacts the offloads,
> >>>+ * so we need to run throught the offload
> >>
> >>throught ?
> >>
> >>>+ * change.
> >>>+ */
> >>>+ rtnl_lock();
> >>>+ ret = set_offload(q, 0);
> >>>+ rtnl_unlock();
> >>>+ }
> >>> q->flags = u;
> >>>+ }
> >>>
> >>> return ret;
> >>>
> >>>@@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file,
> >>>unsigned int cmd,
> >>> TUN_F_TSO_ECN | TUN_F_UFO))
> >>> return -EINVAL;
> >>>
> >>>- /* TODO: only accept frames with the features that
> >>>- got enabled for forwarded frames */
> >>>- if (!(q->flags & IFF_VNET_HDR))
> >>>- return -EINVAL;
> >>> rtnl_lock();
> >>> ret = set_offload(q, arg);
> >>> rtnl_unlock();
> >>>--
> >>>1.8.1.4
> >
^ permalink raw reply
* [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Vlad Yasevich @ 2013-08-14 17:03 UTC (permalink / raw)
To: netdev; +Cc: mst, Vlad Yasevich
When the user turns off IFF_VNET_HDR flag, attempts to change
offload features via TUNSETOFFLOAD do not work. This could cause
GSO packets to be delivered to the user when the user is
not prepared to handle them.
To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
disabled. Treat any attempt to enable offloads as an error in
this case.
We also need to update the TUN_FEATURES mask to include all checksum
options as the underlying device may have something other then
HW_CSUM set.
Change since v1:
- Removed the call to update offloads when IFF_VNET_HDR is turned off.
- Changed the macvtap version of TUN_OFFLOADS to include all checksum
offloads since the physical nic may have them set.
- Treat enabling of offloads without vnet_hdr support as error.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/macvtap.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a98fb0e..3acfc37 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
static const struct proto_ops macvtap_socket_ops;
-#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
+#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
NETIF_F_TSO6 | NETIF_F_UFO)
#define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
/*
@@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
if (!vlan)
return -ENOLINK;
+ /* If the user is trying to set offloads while IFF_VNET_HDR is
+ * off, report it as an error.
+ */
+ if (!(q->flags & IFF_VNET_HDR) && arg)
+ return -EINVAL;
+
features = vlan->dev->features;
if (arg & TUN_F_CSUM) {
@@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
TUN_F_TSO_ECN | TUN_F_UFO))
return -EINVAL;
- /* TODO: only accept frames with the features that
- got enabled for forwarded frames */
- if (!(q->flags & IFF_VNET_HDR))
- return -EINVAL;
rtnl_lock();
ret = set_offload(q, arg);
rtnl_unlock();
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH 1/7] uio: add module owner to prevent inappropriate module unloading
From: Greg Kroah-Hartman @ 2013-08-14 16:33 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior, Hans J. Koch,
Holger Dengler
In-Reply-To: <20130814091946.2643a124@mitra.spranger.biz>
On Wed, Aug 14, 2013 at 09:19:46AM +0200, Benedikt Spranger wrote:
> Am Tue, 13 Aug 2013 10:48:14 -0700
> schrieb Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>
> > On Tue, Aug 13, 2013 at 11:08:36AM +0200, Benedikt Spranger wrote:
> > > If an UIO device is created by another driver (MFD for instance) it has to be
> > > ensured that the MFD driver isn't removed while the UIO is still in use.
> >
> > Shouldn't if the MFD driver is removed, the UIO device will be cleaned
> > up and also removed?
> That is part of the problem:
>
> 1) MFD driver creates platform device "uio_pdrv"
> 2) uio_pdrv creates "UIOX"
> 3) userspace opens "UIOX"
> 4) MFD driver unload (remove platform device "uio_pdrv")
How can this happen in a normal situation? Modules do not simply unload
themselves :)
> 5) userspace reads from "UIOX" --> crash
Step 4 should have told UIO that it was gone and had it shut everything
down properly, so that there would not be a crash.
> > You shouldn't need a module reference for this type of thing.
> The driver uio_pdrv has no chance to recognize that the underlaying platform
> device has gone.
The mfd driver could tell it that it is gone, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net] skge: dma_sync the whole receive buffer
From: Stephen Hemminger @ 2013-08-14 16:20 UTC (permalink / raw)
To: poma; +Cc: David Miller, netdev
In-Reply-To: <520B59D3.4020103@gmail.com>
On Wed, 14 Aug 2013 12:20:03 +0200
poma <pomidorabelisima@gmail.com> wrote:
> On 14.08.2013 03:00, Stephen Hemminger wrote:
> > On Tue, 13 Aug 2013 15:09:55 -0700 (PDT)
> > David Miller <davem@davemloft.net> wrote:
> >
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Date: Sat, 10 Aug 2013 15:02:07 -0700
> >>
> >>> The DMA sync should sync the whole receive buffer, not just
> >>> part of it. Fixes log messages dma_sync_check.
> >>>
> >>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >>
> >> Applied, but I really suspect that your "check DMA mapping errors"
> >> patch has added a serious regression. A regression much worse than
> >> the bug you were trying to fix with that change.
> >
> > Argh. The problem is deeper than that. Device got broken somewhere between
> > 3.2 and 3.4. My old Dlink card works on 3.2 but gets DMA errors on 3.4.
> > The config's are different though so checking that as well.
> >
>
> Can I help you with debugging?
> DGE-530T is rather solid device.
Don't think it is a hardware problem.
The failure is when the board access the Receive ring PCI memory area.
This region is allocated with pci_alloc_consistent and therefore should
be available. Two possible issues are driver math issues, or hardware
problems with where the region is located. Some of these cards don't
really have full 64 bit PCI support.
My board is:
05:01.0 Ethernet controller: D-Link System Inc Gigabit Ethernet Adapter (rev 11)
Subsystem: D-Link System Inc DGE-530T Gigabit Ethernet Adapter
Flags: bus master, 66MHz, medium devsel, latency 32, IRQ 18
Memory at f7d20000 (32-bit, non-prefetchable) [size=16K]
I/O ports at c000 [size=256]
Expansion ROM at f7d00000 [disabled] [size=128K]
Capabilities: [48] Power Management version 2
Capabilities: [50] Vital Product Data
Kernel driver in use: skge
What is your config?
^ permalink raw reply
* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Vlad Yasevich @ 2013-08-14 16:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <520B8F05.6060402@redhat.com>
On 08/14/2013 10:07 AM, Vlad Yasevich wrote:
> On 08/14/2013 08:10 AM, Michael S. Tsirkin wrote:
>> On Tue, Aug 13, 2013 at 01:55:58PM -0400, Vlad Yasevich wrote:
>>> When the user turns off IFF_VNET_HDR flag, attempts to change
>>> offload features via TUNSETOFFLOAD do not work. This could cause
>>> GSO packets to be delivered to the user when the user is
>>> not prepared to handle them.
>>>
>>> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
>>> disabled and make sure to turn off all offloads in this case.
>>> Also, when IFF_VNET_HDR is disabled, run throught the offload change
>>> as well to make sure that the functionality is not dependent on
>>> the order of the ioclt() calls.
>>>
>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>
>> Hmm, this seems to make it even messier.
>> So
>> set TUNSETOFFLOAD
>> clear IFF_VNET_HDR
>> set IFF_VNET_HDR
>>
>> offloads are now clear?
>>
>> I'm also not sure userspace that sets offloads but clears
>> vnet hdr knows what it's doing.
>>
>> How about we fail attempts to clear vnet hdr
>> unless offloads are disabled?
>>
>
> OK. That makes it simpler...
Spoke too soon.
What happens is that libvirt is typically the one that turns off
IFF_VNET_HDR, and qemu controls the offloads.
So, if we forbid turning off vnet_hdr while offloads are set, that
will break libvirt (this is because macvtap defaults to tap offloads
being enabled).
-vlad
>
> -vlad
>
>>
>>
>>
>>> ---
>>> drivers/net/macvtap.c | 27 +++++++++++++++++++--------
>>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>>> index a98fb0e..076b9e7 100644
>>> --- a/drivers/net/macvtap.c
>>> +++ b/drivers/net/macvtap.c
>>> @@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q,
>>> unsigned long arg)
>>> struct macvlan_dev *vlan;
>>> netdev_features_t features;
>>> netdev_features_t feature_mask = 0;
>>> + netdev_features_t tap_mask = TUN_OFFLOADS;
>>>
>>> vlan = rtnl_dereference(q->vlan);
>>> if (!vlan)
>>> @@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue
>>> *q, unsigned long arg)
>>>
>>> features = vlan->dev->features;
>>>
>>> - if (arg & TUN_F_CSUM) {
>>> + if (!(q->flags & IFF_VNET_HDR)) {
>>> + /* Turn off all checsum offloading also if user does
>>> + * not user vnet_hdr.
>>
>> Does not set?
>>
>>> + */
>>> + tap_mask |= NETIF_F_ALL_CSUM;
>>> + } else if (arg & TUN_F_CSUM) {
>>> feature_mask = NETIF_F_HW_CSUM;
>>>
>>> if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
>>> @@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q,
>>> unsigned long arg)
>>> /* tap_features are the same as features on tun/tap and
>>> * reflect user expectations.
>>> */
>>> - vlan->tap_features = vlan->dev->features &
>>> - (feature_mask | ~TUN_OFFLOADS);
>>> + vlan->tap_features = vlan->dev->features & (feature_mask |
>>> ~tap_mask);
>>> vlan->set_features = features;
>>> netdev_update_features(vlan->dev);
>>>
>>> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file,
>>> unsigned int cmd,
>>> if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
>>> (IFF_NO_PI | IFF_TAP))
>>> ret = -EINVAL;
>>> - else
>>> + else {
>>> + if ((q->flags ^ u) & IFF_VNET_HDR) {
>>> + /* vnet_hdr support impacts the offloads,
>>> + * so we need to run throught the offload
>>
>> throught ?
>>
>>> + * change.
>>> + */
>>> + rtnl_lock();
>>> + ret = set_offload(q, 0);
>>> + rtnl_unlock();
>>> + }
>>> q->flags = u;
>>> + }
>>>
>>> return ret;
>>>
>>> @@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file,
>>> unsigned int cmd,
>>> TUN_F_TSO_ECN | TUN_F_UFO))
>>> return -EINVAL;
>>>
>>> - /* TODO: only accept frames with the features that
>>> - got enabled for forwarded frames */
>>> - if (!(q->flags & IFF_VNET_HDR))
>>> - return -EINVAL;
>>> rtnl_lock();
>>> ret = set_offload(q, arg);
>>> rtnl_unlock();
>>> --
>>> 1.8.1.4
>
^ permalink raw reply
* Re: [PATCH] Revert "cxgb3: Check and handle the dma mapping errors"
From: Divy Le ray @ 2013-08-14 15:57 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Linus Torvalds, Santosh Rastapur,
Jay Fenlason, David S. Miller
In-Reply-To: <1376471941-2914-1-git-send-email-aik@ozlabs.ru>
On 08/14/2013 02:19 AM, Alexey Kardashevskiy wrote:
> This reverts commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9.
>
> As the tests PPC64 (powernv platform) show, IOMMU pages are leaking
> when transferring big amount of small packets (<=64 bytes),
> "ping -f" and waiting for 15 seconds is the simplest way to confirm the bug.
>
> Cc: Linus Torvalds<torvalds@linux-foundation.org>
> Cc: Santosh Rastapur<santosh@chelsio.com>
> Cc: Jay Fenlason<fenlason@redhat.com>
> Cc: David S. Miller<davem@davemloft.net>
> Cc: Divy Le ray<divy@chelsio.com>
> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
Acked-by: Divy Le Ray <divy@chelsio.com>
We are revisiting this patch in the light of the leak, and will repost
once fixed.
Cheers,
Divy
> ---
> drivers/net/ethernet/chelsio/cxgb3/sge.c | 107 +++++++------------------------
> 1 file changed, 24 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c b/drivers/net/ethernet/chelsio/cxgb3/sge.c
> index 687ec4a..9c89dc8 100644
> --- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
> +++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
> @@ -455,11 +455,6 @@ static int alloc_pg_chunk(struct adapter *adapter, struct sge_fl *q,
> q->pg_chunk.offset = 0;
> mapping = pci_map_page(adapter->pdev, q->pg_chunk.page,
> 0, q->alloc_size, PCI_DMA_FROMDEVICE);
> - if (unlikely(pci_dma_mapping_error(adapter->pdev, mapping))) {
> - __free_pages(q->pg_chunk.page, order);
> - q->pg_chunk.page = NULL;
> - return -EIO;
> - }
> q->pg_chunk.mapping = mapping;
> }
> sd->pg_chunk = q->pg_chunk;
> @@ -954,75 +949,40 @@ static inline unsigned int calc_tx_descs(const struct sk_buff *skb)
> return flits_to_desc(flits);
> }
>
> -
> -/* map_skb - map a packet main body and its page fragments
> - * @pdev: the PCI device
> - * @skb: the packet
> - * @addr: placeholder to save the mapped addresses
> - *
> - * map the main body of an sk_buff and its page fragments, if any.
> - */
> -static int map_skb(struct pci_dev *pdev, const struct sk_buff *skb,
> - dma_addr_t *addr)
> -{
> - const skb_frag_t *fp, *end;
> - const struct skb_shared_info *si;
> -
> - *addr = pci_map_single(pdev, skb->data, skb_headlen(skb),
> - PCI_DMA_TODEVICE);
> - if (pci_dma_mapping_error(pdev, *addr))
> - goto out_err;
> -
> - si = skb_shinfo(skb);
> - end = &si->frags[si->nr_frags];
> -
> - for (fp = si->frags; fp < end; fp++) {
> - *++addr = skb_frag_dma_map(&pdev->dev, fp, 0, skb_frag_size(fp),
> - DMA_TO_DEVICE);
> - if (pci_dma_mapping_error(pdev, *addr))
> - goto unwind;
> - }
> - return 0;
> -
> -unwind:
> - while (fp-- > si->frags)
> - dma_unmap_page(&pdev->dev, *--addr, skb_frag_size(fp),
> - DMA_TO_DEVICE);
> -
> - pci_unmap_single(pdev, addr[-1], skb_headlen(skb), PCI_DMA_TODEVICE);
> -out_err:
> - return -ENOMEM;
> -}
> -
> /**
> - * write_sgl - populate a scatter/gather list for a packet
> + * make_sgl - populate a scatter/gather list for a packet
> * @skb: the packet
> * @sgp: the SGL to populate
> * @start: start address of skb main body data to include in the SGL
> * @len: length of skb main body data to include in the SGL
> - * @addr: the list of the mapped addresses
> + * @pdev: the PCI device
> *
> - * Copies the scatter/gather list for the buffers that make up a packet
> + * Generates a scatter/gather list for the buffers that make up a packet
> * and returns the SGL size in 8-byte words. The caller must size the SGL
> * appropriately.
> */
> -static inline unsigned int write_sgl(const struct sk_buff *skb,
> +static inline unsigned int make_sgl(const struct sk_buff *skb,
> struct sg_ent *sgp, unsigned char *start,
> - unsigned int len, const dma_addr_t *addr)
> + unsigned int len, struct pci_dev *pdev)
> {
> - unsigned int i, j = 0, k = 0, nfrags;
> + dma_addr_t mapping;
> + unsigned int i, j = 0, nfrags;
>
> if (len) {
> + mapping = pci_map_single(pdev, start, len, PCI_DMA_TODEVICE);
> sgp->len[0] = cpu_to_be32(len);
> - sgp->addr[j++] = cpu_to_be64(addr[k++]);
> + sgp->addr[0] = cpu_to_be64(mapping);
> + j = 1;
> }
>
> nfrags = skb_shinfo(skb)->nr_frags;
> for (i = 0; i < nfrags; i++) {
> const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> + mapping = skb_frag_dma_map(&pdev->dev, frag, 0, skb_frag_size(frag),
> + DMA_TO_DEVICE);
> sgp->len[j] = cpu_to_be32(skb_frag_size(frag));
> - sgp->addr[j] = cpu_to_be64(addr[k++]);
> + sgp->addr[j] = cpu_to_be64(mapping);
> j ^= 1;
> if (j == 0)
> ++sgp;
> @@ -1178,7 +1138,7 @@ static void write_tx_pkt_wr(struct adapter *adap, struct sk_buff *skb,
> const struct port_info *pi,
> unsigned int pidx, unsigned int gen,
> struct sge_txq *q, unsigned int ndesc,
> - unsigned int compl, const dma_addr_t *addr)
> + unsigned int compl)
> {
> unsigned int flits, sgl_flits, cntrl, tso_info;
> struct sg_ent *sgp, sgl[MAX_SKB_FRAGS / 2 + 1];
> @@ -1236,7 +1196,7 @@ static void write_tx_pkt_wr(struct adapter *adap, struct sk_buff *skb,
> }
>
> sgp = ndesc == 1 ? (struct sg_ent *)&d->flit[flits] : sgl;
> - sgl_flits = write_sgl(skb, sgp, skb->data, skb_headlen(skb), addr);
> + sgl_flits = make_sgl(skb, sgp, skb->data, skb_headlen(skb), adap->pdev);
>
> write_wr_hdr_sgl(ndesc, skb, d, pidx, q, sgl, flits, sgl_flits, gen,
> htonl(V_WR_OP(FW_WROPCODE_TUNNEL_TX_PKT) | compl),
> @@ -1267,7 +1227,6 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
> struct netdev_queue *txq;
> struct sge_qset *qs;
> struct sge_txq *q;
> - dma_addr_t addr[MAX_SKB_FRAGS + 1];
>
> /*
> * The chip min packet length is 9 octets but play safe and reject
> @@ -1296,11 +1255,6 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
> return NETDEV_TX_BUSY;
> }
>
> - if (unlikely(map_skb(adap->pdev, skb, addr) < 0)) {
> - dev_kfree_skb(skb);
> - return NETDEV_TX_OK;
> - }
> -
> q->in_use += ndesc;
> if (unlikely(credits - ndesc < q->stop_thres)) {
> t3_stop_tx_queue(txq, qs, q);
> @@ -1358,7 +1312,7 @@ netdev_tx_t t3_eth_xmit(struct sk_buff *skb, struct net_device *dev)
> if (likely(!skb_shared(skb)))
> skb_orphan(skb);
>
> - write_tx_pkt_wr(adap, skb, pi, pidx, gen, q, ndesc, compl, addr);
> + write_tx_pkt_wr(adap, skb, pi, pidx, gen, q, ndesc, compl);
> check_ring_tx_db(adap, q);
> return NETDEV_TX_OK;
> }
> @@ -1623,8 +1577,7 @@ static void setup_deferred_unmapping(struct sk_buff *skb, struct pci_dev *pdev,
> */
> static void write_ofld_wr(struct adapter *adap, struct sk_buff *skb,
> struct sge_txq *q, unsigned int pidx,
> - unsigned int gen, unsigned int ndesc,
> - const dma_addr_t *addr)
> + unsigned int gen, unsigned int ndesc)
> {
> unsigned int sgl_flits, flits;
> struct work_request_hdr *from;
> @@ -1645,9 +1598,9 @@ static void write_ofld_wr(struct adapter *adap, struct sk_buff *skb,
>
> flits = skb_transport_offset(skb) / 8;
> sgp = ndesc == 1 ? (struct sg_ent *)&d->flit[flits] : sgl;
> - sgl_flits = write_sgl(skb, sgp, skb_transport_header(skb),
> - skb_tail_pointer(skb) -
> - skb_transport_header(skb), addr);
> + sgl_flits = make_sgl(skb, sgp, skb_transport_header(skb),
> + skb->tail - skb->transport_header,
> + adap->pdev);
> if (need_skb_unmap()) {
> setup_deferred_unmapping(skb, adap->pdev, sgp, sgl_flits);
> skb->destructor = deferred_unmap_destructor;
> @@ -1705,11 +1658,6 @@ again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> goto again;
> }
>
> - if (map_skb(adap->pdev, skb, (dma_addr_t *)skb->head)) {
> - spin_unlock(&q->lock);
> - return NET_XMIT_SUCCESS;
> - }
> -
> gen = q->gen;
> q->in_use += ndesc;
> pidx = q->pidx;
> @@ -1720,7 +1668,7 @@ again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> }
> spin_unlock(&q->lock);
>
> - write_ofld_wr(adap, skb, q, pidx, gen, ndesc, (dma_addr_t *)skb->head);
> + write_ofld_wr(adap, skb, q, pidx, gen, ndesc);
> check_ring_tx_db(adap, q);
> return NET_XMIT_SUCCESS;
> }
> @@ -1738,7 +1686,6 @@ static void restart_offloadq(unsigned long data)
> struct sge_txq *q = &qs->txq[TXQ_OFLD];
> const struct port_info *pi = netdev_priv(qs->netdev);
> struct adapter *adap = pi->adapter;
> - unsigned int written = 0;
>
> spin_lock(&q->lock);
> again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> @@ -1758,14 +1705,10 @@ again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> break;
> }
>
> - if (map_skb(adap->pdev, skb, (dma_addr_t *)skb->head))
> - break;
> -
> gen = q->gen;
> q->in_use += ndesc;
> pidx = q->pidx;
> q->pidx += ndesc;
> - written += ndesc;
> if (q->pidx >= q->size) {
> q->pidx -= q->size;
> q->gen ^= 1;
> @@ -1773,8 +1716,7 @@ again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> __skb_unlink(skb, &q->sendq);
> spin_unlock(&q->lock);
>
> - write_ofld_wr(adap, skb, q, pidx, gen, ndesc,
> - (dma_addr_t *)skb->head);
> + write_ofld_wr(adap, skb, q, pidx, gen, ndesc);
> spin_lock(&q->lock);
> }
> spin_unlock(&q->lock);
> @@ -1784,9 +1726,8 @@ again: reclaim_completed_tx(adap, q, TX_RECLAIM_CHUNK);
> set_bit(TXQ_LAST_PKT_DB, &q->flags);
> #endif
> wmb();
> - if (likely(written))
> - t3_write_reg(adap, A_SG_KDOORBELL,
> - F_SELEGRCNTX | V_EGRCNTX(q->cntxt_id));
> + t3_write_reg(adap, A_SG_KDOORBELL,
> + F_SELEGRCNTX | V_EGRCNTX(q->cntxt_id));
> }
>
> /**
^ permalink raw reply
* [PATCH] fixing adaptive-red average queuing in man/man8/tc-red.8
From: Naeem Khademi @ 2013-08-14 15:51 UTC (permalink / raw)
To: netdev, stephen
This patch fixes a mistake in documenting adaptive-RED's formula of
average queuing in man. The ARED's target average queuing is
(max+min)/2.
Regards,
diff --git a/man/man8/tc-red.8 b/man/man8/tc-red.8
index f410d15..466000d 100644
--- a/man/man8/tc-red.8
+++ b/man/man8/tc-red.8
@@ -127,7 +127,7 @@ adaptive
(Added in linux-3.3) Sets RED in adaptive mode as described in
http://icir.org/floyd/papers/adaptiveRed.pdf
.nf
Goal of Adaptive RED is to make 'probability' dynamic value between
1% and 50% to reach the target average queue :
-.B (max - min) / 2
+.B (max + min) / 2
.fi
.SH EXAMPLE
^ permalink raw reply related
* [PATCH net-next 2/2] ipv4: processing ancillary IP_TOS or IP_TTL
From: Francesco Fusco @ 2013-08-14 15:48 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <cover.1376494030.git.ffusco@redhat.com>
If IP_TOS or IP_TTL are specified as ancillary data, then sendmsg() sends out
packets with the specified TTL or TOS overriding the socket values specified
with the traditional setsockopt().
If there is a per-packet specified tos, the skb->priority is set accordingly.
The ipv4_is_multicast() function is used to fill in the right TTL value in case
of multicast destinations.
Signed-off-by: Francesco Fusco <ffusco@redhat.com>
---
include/net/inet_sock.h | 3 +++
net/ipv4/icmp.c | 11 ++++++-----
net/ipv4/ip_output.c | 17 ++++++++++++++---
net/ipv4/raw.c | 3 +++
net/ipv4/udp.c | 3 +++
5 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index b21a7f0..5e22c9e 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -101,6 +101,9 @@ struct inet_cork {
struct ip_options *opt;
unsigned int fragsize;
int length; /* Total length of all frames */
+ __s16 ttl;
+ __s16 tos;
+ __u32 priority;
struct dst_entry *dst;
u8 tx_flags;
};
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 5f7d11a..28bccce 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -348,9 +348,11 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
icmp_param->data.icmph.checksum = 0;
- inet->tos = ip_hdr(skb)->tos;
daddr = ipc.addr = ip_hdr(skb)->saddr;
saddr = fib_compute_spec_dst(skb);
+ ipc.tos = ip_hdr(skb)->tos;
+ ipc.ttl = ipv4_is_multicast(daddr) ? inet->mc_ttl : inet->uc_ttl;
+ ipc.priority = sk->sk_priority;
ipc.opt = NULL;
ipc.tx_flags = 0;
if (icmp_param->replyopts.opt.opt.optlen) {
@@ -487,7 +489,6 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
struct ipcm_cookie ipc;
struct flowi4 fl4;
__be32 saddr;
- u8 tos;
struct net *net;
struct sock *sk;
@@ -586,7 +587,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
rcu_read_unlock();
}
- tos = icmp_pointers[type].error ? ((iph->tos & IPTOS_TOS_MASK) |
+ ipc.tos = icmp_pointers[type].error ? ((iph->tos & IPTOS_TOS_MASK) |
IPTOS_PREC_INTERNETCONTROL) :
iph->tos;
@@ -604,12 +605,12 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
icmp_param->data.icmph.checksum = 0;
icmp_param->skb = skb_in;
icmp_param->offset = skb_network_offset(skb_in);
- inet_sk(sk)->tos = tos;
+ ipc.ttl = -1;
ipc.addr = iph->saddr;
ipc.opt = &icmp_param->replyopts.opt;
ipc.tx_flags = 0;
- rt = icmp_route_lookup(net, &fl4, skb_in, iph, saddr, tos,
+ rt = icmp_route_lookup(net, &fl4, skb_in, iph, saddr, ipc.tos,
type, code, icmp_param);
if (IS_ERR(rt))
goto out_unlock;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 4bcabf3..bf46730 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1068,6 +1068,9 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
rt->dst.dev->mtu : dst_mtu(&rt->dst);
cork->dst = &rt->dst;
cork->length = 0;
+ cork->ttl = ipc->ttl;
+ cork->tos = ipc->tos;
+ cork->priority = ipc->priority;
cork->tx_flags = ipc->tx_flags;
return 0;
@@ -1319,7 +1322,9 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
if (cork->flags & IPCORK_OPT)
opt = cork->opt;
- if (rt->rt_type == RTN_MULTICAST)
+ if (cork->ttl != -1)
+ ttl = cork->ttl;
+ else if (rt->rt_type == RTN_MULTICAST)
ttl = inet->mc_ttl;
else
ttl = ip_select_ttl(inet, &rt->dst);
@@ -1327,7 +1332,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
iph = (struct iphdr *)skb->data;
iph->version = 4;
iph->ihl = 5;
- iph->tos = inet->tos;
+ iph->tos = (cork->tos != inet->tos) ? cork->tos : inet->tos;
iph->frag_off = df;
iph->ttl = ttl;
iph->protocol = sk->sk_protocol;
@@ -1339,7 +1344,8 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
ip_options_build(skb, opt, cork->addr, rt, 0);
}
- skb->priority = sk->sk_priority;
+ skb->priority = (cork->tos != inet->tos) ?
+ cork->priority : sk->sk_priority;
skb->mark = sk->sk_mark;
/*
* Steal rt from cork.dst to avoid a pair of atomic_inc/atomic_dec
@@ -1511,6 +1517,11 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
inet = &get_cpu_var(unicast_sock);
inet->tos = arg->tos;
+
+ ipc.tos = ip_hdr(skb)->tos;
+ ipc.ttl = inet->uc_ttl;
+ ipc.priority = skb->priority;
+
sk = &inet->sk;
sk->sk_priority = skb->priority;
sk->sk_protocol = ip_hdr(skb)->protocol;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index dd44e0a..68a9423 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -518,6 +518,9 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
ipc.opt = NULL;
ipc.tx_flags = 0;
ipc.oif = sk->sk_bound_dev_if;
+ ipc.ttl = ipv4_is_multicast(daddr) ? inet->mc_ttl : inet->uc_ttl;
+ ipc.tos = inet->tos;
+ ipc.priority = sk->sk_priority;
if (msg->msg_controllen) {
err = ip_cmsg_send(sock_net(sk), msg, &ipc);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 9e88af0..2d72ce8 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -904,6 +904,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
}
ipc.addr = inet->inet_saddr;
+ ipc.ttl = ipv4_is_multicast(daddr) ? inet->mc_ttl : inet->uc_ttl;
+ ipc.tos = inet->tos;
+ ipc.priority = sk->sk_priority;
ipc.oif = sk->sk_bound_dev_if;
sock_tx_timestamp(sk, &ipc.tx_flags);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/2] ipv4: IP_TOS and IP_TTL can be specified as ancillary data
From: Francesco Fusco @ 2013-08-14 15:48 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <cover.1376494030.git.ffusco@redhat.com>
This patch enables the IP_TTL and IP_TOS values passed from userspace to
be stored in the ipcm_cookie struct.
Signed-off-by: Francesco Fusco <ffusco@redhat.com>
---
include/net/ip.h | 3 +++
net/ipv4/ip_sockglue.c | 20 +++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index a68f838..bf63936 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -54,6 +54,9 @@ static inline unsigned int ip_hdrlen(const struct sk_buff *skb)
struct ipcm_cookie {
__be32 addr;
int oif;
+ __s16 ttl;
+ __s16 tos;
+ __u32 priority;
struct ip_options_rcu *opt;
__u8 tx_flags;
};
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index d9c4f11..56e3445 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -189,7 +189,7 @@ EXPORT_SYMBOL(ip_cmsg_recv);
int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc)
{
- int err;
+ int err, val;
struct cmsghdr *cmsg;
for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
@@ -215,6 +215,24 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc)
ipc->addr = info->ipi_spec_dst.s_addr;
break;
}
+ case IP_TTL:
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
+ return -EINVAL;
+ val = *(int *)CMSG_DATA(cmsg);
+ if (val < 1 || val > 255)
+ return -EINVAL;
+ ipc->ttl = val;
+ break;
+ case IP_TOS:
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
+ return -EINVAL;
+ val = *(int *)CMSG_DATA(cmsg);
+ if (val < 0 || val > 255)
+ return -EINVAL;
+ ipc->tos = val;
+ ipc->priority = rt_tos2priority(ipc->tos);
+ break;
+
default:
return -EINVAL;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/2] ipv4: per-datagram IP_TOS and IP_TTL via sendmsg()
From: Francesco Fusco @ 2013-08-14 15:48 UTC (permalink / raw)
To: davem; +Cc: netdev
There is no way to set the IP_TOS field on a per-packet basis in IPv4, while
IPv6 has such a mechanism. Therefore one has to fall back to the setsockopt()
in case of IPv4.
Using the existing per-socket option is not convenient particularly in the
situations where multiple threads have to use the same socket data requiring
per-thread TOS values. In fact this would involve calling setsockopt() before
sendmsg() every time.
An old patch was submitted to the netdev mailing list to add support for
setting TOS on a per-packet basis, but was never integrated into the kernel.
The old patch can be found here:
http://marc.info/?l=linux-netdev&m=118388241201593
This patch contains the changes from the original patch, updated for the
current kernel.
Francesco Fusco (2):
ipv4: IP_TOS and IP_TTL can be specified as ancillary data
ipv4: processing ancillary IP_TOS or IP_TTL
include/net/inet_sock.h | 3 +++
include/net/ip.h | 3 +++
net/ipv4/icmp.c | 11 ++++++-----
net/ipv4/ip_output.c | 17 ++++++++++++++---
net/ipv4/ip_sockglue.c | 20 +++++++++++++++++++-
net/ipv4/raw.c | 3 +++
net/ipv4/udp.c | 3 +++
7 files changed, 51 insertions(+), 9 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: Configuring and using RIPngd
From: Ale Lampro @ 2013-08-14 15:34 UTC (permalink / raw)
To: Phil Oester; +Cc: netdev
In-Reply-To: <20130814152424.GA10099@linuxace.com>
Hi Phil,
Thanks, you are totally right.
On Wed, Aug 14, 2013 at 5:24 PM, Phil Oester <kernel@linuxace.com> wrote:
> On Wed, Aug 14, 2013 at 12:02:35PM +0200, Ale Lampro wrote:
>> Hi,
>>
>> I am new to the list and I am trying to achieve something that I think
>> should be relatively easy with RIPngd and Quagga, but I am new to all
>> this so I have some issues.
>
> Best to ask this question on the Quagga mailing list.
>
> Phil
^ permalink raw reply
* Re: [PATCH v3 00/11] Add namespace support for syslog
From: Serge E. Hallyn @ 2013-08-14 15:30 UTC (permalink / raw)
To: Rui Xiang
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
libo.chen-hv44wF8Li93QT0dZR+AlfA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
guz.fnst-BthXqXjhjHXQFUHtdCDX3A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Eric W. Biederman
In-Reply-To: <52037D50.2050109-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Quoting Rui Xiang (rui.xiang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org):
> On 2013/8/8 9:37, Gao feng wrote:
> > On 08/07/2013 03:55 PM, Eric W. Biederman wrote:
> >>
> >> Since this still has not been addressed. I am going to repeat Andrews
> >> objection again.
> >>
> >> Isn't there a better way to get iptables information out than to use
> >> syslog. I did not have time to follow up on that but it did appear that
> >> someone did have a better way to get the information out.
> >>
> >> Essentially the argument against this goes. The kernel logging facility
> >> is really not a particularly good tool to be using for anything other
> >> than kernel debugging information, and there appear to be no substantial
> >> uses for a separate syslog that should not be done in other ways.
> >
> > containerizing syslog is not only for iptables, it also isolates the /dev/kmsg,
> > /proc/kmsg, syslog(2)... user space tools in container may use this interface
> > to read/generate syslog.
> >
> > But I don't know how important/urgent this containerizing syslog work is,
> > Rui Xiang, can you find an important/popular user space tool which uses this
> > interfaces to generate kernel syslog?
> >
>
> There are some other cases. Some warnings (bad mount options for tmpfs,
> bad uid owner for many of them, etc) emerged in the container should
> be exported to the container. Some belong on the host - if they show
> a corrupt superblock which may indicate an attempt by the container
> to crash the kernel. Like these, Kernel will print out warnings when
> userspace in container uses a deprecated something or other, and these
> logs should be invisible and specific for current container.
>
> I can't say this work is terribly compelling and important, but the
> impact may be obvious, IMO.
Aug 9 21:49:13 sergeh1 kernel: [4644829.672768] init: Failed to spawn network-interface (veth8Ehlvj) post-stop process: unable to change root directory: No such file ricr:aeohgrticr cfe rty444984 n:aetswnw-ta(ht -rrsultheoit: hlrro<4865i:i sntkta(ht ttpe btheoit: hlrrob r6ezt)nrgoadgte644915 c0pt(tyg ti wd a
Aug 9 21:49:13 sergeh1 kernel: X3f d-6:uigitra ora
Aug 9 22:19:54 sergeh1 kernel: 6[642.175 X3f d-6:mutdflsse ihodrddt oe==99 rfl=lccnanrdfutwt-etn"nm=/a/ah/x/lu-ui/m.AExdu"pi=91 om=mut sye"x3name="/devlo0" lg=r"ol pmc r=3an19pfel-nireu-tntgne/rc/cldudmHEqu d97o=otfy=x"ra=d/o/fg""8b:o vhc)nrgibdte646013 veeMzWe oso d<[4715]xr r1eMc egset
Aug
^ permalink raw reply
* Re: [PATCH v2 net-next 2/6] vlan: add __vlan_find_dev_next()
From: Veaceslav Falico @ 2013-08-14 15:28 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, Patrick McHardy, David S. Miller
In-Reply-To: <5204CD73.9010309@redhat.com>
On Fri, Aug 09, 2013 at 01:07:31PM +0200, Nikolay Aleksandrov wrote:
...snip...
>We already discussed privately my comments about this new function, I'll leave
>them here just for the sake of having them documented:
As an almost top-posting - I'm currently on my vacation, so just a few
comments, without real code/patches. I'll come up with v3 at the next week,
hopefully.
>My proposition is to drop these changes altogether (the new function, and the
>vlan.h macro change) and instead add something like the following (I haven't
>tested it, it's only to illustrate the idea) in if_vlan.h so it'll be accessible
>to everyone:
>
>Version 1 (circular so you can simplify the bonding ALB code, I haven't left
>spaces because of auto-wrapping):
>
>#define dev_for_each_vlan_from(dev, vlandev, proto, from, i)
> for (i=from+1, vlandev=__vlan_find_dev_deep(dev, proto, from); \
> i!=from; \
> i=(i+1)%VLAN_N_VID, vlandev=__vlan_find_dev_deep(dev, proto, i)) \
> if (vlandev)
>
>#define dev_for_each_vlan(dev, vlandev, proto, i)
> dev_for_each_vlan_from(dev, vlandev, proto, 0, i)
I like this idea, however it's an overkill in some places (and calling
8k times __vlan_find_dev_deep()->rcu_dereference, verify for vlan_info,
re-call itself 8k times if it's really a slave etc... not really good).
That was one of the main thoughts when I've chosen the other way -
while (vlan = vlan_next(vlan))
They're also quite ugly (huge for, and an if () inside of a nested define)
- but that's more an IMO.
But, again, I like the dev_for_each_vlan() idea, and will look at it again,
trying to omit the __vlan_find_dev_deep() somehow and without that
monstrous for() :).
>
>Version 2 is the same but a little shorter, without the circular part.
>This way you reuse the already provided function __vlan_find_dev_deep and the
>churn is smaller, also __vlan_find_dev_deep takes care of the master issue (that
>is if dev doesn't have a vlan_info, then its master's vlan_info will be used).
Great ideas, thank you.
>Also the code will look much nicer IMO changing this:
>while ((vlan_dev = __vlan_find_dev_next))
>
>to
>
>dev_for_each_vlan(dev, vlan_dev, proto, i)
Yep, I agree, dev_for_each_vlan() looks a lot more readable. Maybe the
proto and i args can also be dropped somehow, I'll take a look...
>
>There're also 2 nice side-effects, first you'll only walk over 4096 entries (for
>the specified vlan proto only)
Not a big one, tbh, cause the current code looks for at most
vlan_id..8192(+0..vlan_id), so 8192. But the proto is indeed a good
side-effect. Another point - we're not working with QinQ at all...
>and the bonding ALB code will simplify from the
>ambiguous looking:
> vlan_id = bond->alb_info.current_alb_vlan;
> vlan_dev = __vlan_find_dev_deep(bond->dev,
> htons(ETH_P_8021Q),
> vlan_id);
>
> /* search for the next one, if not found - for any */
> if (vlan_dev)
> vlan_dev = __vlan_find_dev_next(bond->dev,
> vlan_dev);
> if (!vlan_dev)
> vlan_dev = __vlan_find_dev_next(bond->dev,
> NULL);
>
> if (vlan_dev) {
> vlan_id = vlan_dev_vlan_id(vlan_dev);
> bond->alb_info.current_alb_vlan = vlan_id;
> } else {
> bond->alb_info.current_alb_vlan = 0;
> rcu_read_unlock();
> kfree_skb(skb);
> continue;
> }
>
>to something like (again untested, sorry for the style - line wrapping):
Yep, that's why I really like the idea with dev_for_each_vlan(), with one
minor comment (which is fixable, I guess).
>
> vlan_id = bond->alb_info.current_alb_vlan+1;
> /* since from here is current+1, if there aren't any
> * vlans up to current, it'll get current again if it's
> * available
> */
> dev_for_each_vlan_from(bond->dev, vlan_dev, htons(ETH_P_8021Q), vlan_id, i)
> break;
^^^^^ THIS. It makes a big "WHAT?" in my head. :)
>
> if (vlan_dev) {
> vlan_id = vlan_dev_vlan_id(vlan_dev);
> bond->alb_info.current_alb_vlan = vlan_id;
> } else {
> bond->alb_info.current_alb_vlan = 0;
> rcu_read_unlock();
> kfree_skb(skb);
> continue;
> }
>
>Cheers,
> Nik
Thanks a lot for the feedback, really appreciated :).
^ 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