* Re: [PATCH v2 net-next] 8390 : Replace ei_debug with msg_enable/NETIF_MSG_* feature
From: Joe Perches @ 2013-11-08 19:01 UTC (permalink / raw)
To: Matthew Whitehead; +Cc: netdev
In-Reply-To: <1383935522-19200-1-git-send-email-tedheadster@gmail.com>
On Fri, 2013-11-08 at 13:32 -0500, Matthew Whitehead wrote:
> Removed the shared ei_debug variable. Replaced it by adding u32 msg_enable to
> the private struct ei_device. Now each 8390 ethernet instance has a per-device
> logging variable.
>
> Changed older style printk() calls to more canonical forms.
Hi again Matthew:
You're going to have to resend this in a few weeks as
net-next is not currently open for non-regression
patches. Right now is what's called a "merge window"
after the last "official" release and before the
"release candidate" releases. Most of the development
trees are kept unchanged/solidified before at this
point in time.
Now, just some trivial notes:
> diff --git a/drivers/net/ethernet/8390/apne.c b/drivers/net/ethernet/8390/apne.c
[]
> @@ -133,11 +139,11 @@ struct net_device * __init apne_probe(int unit)
> if ( !(AMIGAHW_PRESENT(PCMCIA)) )
> return ERR_PTR(-ENODEV);
>
> - printk("Looking for PCMCIA ethernet card : ");
> + printk(KERN_INFO "Looking for PCMCIA ethernet card : ");
>
> /* check if a card is inserted */
> if (!(PCMCIA_INSERTED)) {
> - printk("NO PCMCIA card inserted\n");
> + printk(KERN_INFO "NO PCMCIA card inserted\n");
This should be pr_cont/printk(KERN_CONT
Please check whether or not each of the below printks is
actually a continuation of this "looking for..." or is a
standalone line.
Mark the continuations as pr_cont/KERN_CONT.
> return ERR_PTR(-ENODEV);
> }
>
> @@ -148,6 +154,8 @@ struct net_device * __init apne_probe(int unit)
> sprintf(dev->name, "eth%d", unit);
> netdev_boot_setup_check(dev);
> }
> + ei_local = netdev_priv(dev);
> + ei_local->msg_enable = apne_debug;
>
> /* disable pcmcia irq for readtuple */
> pcmcia_disable_irq();
> @@ -155,14 +163,14 @@ struct net_device * __init apne_probe(int unit)
> #ifndef MANUAL_CONFIG
> if ((pcmcia_copy_tuple(CISTPL_FUNCID, tuple, 8) < 3) ||
> (tuple[2] != CISTPL_FUNCID_NETWORK)) {
> - printk("not an ethernet card\n");
> + netdev_info(dev, "not an ethernet card\n");
likely this should be pr_cont too.
> - printk("ethernet PCMCIA card inserted\n");
> + netdev_info(dev, "ethernet PCMCIA card inserted\n");
another pr_cont
etc...
> if (!init_pcmcia()) {
> /* XXX: shouldn't we re-enable irq here? */
> @@ -204,11 +212,12 @@ static int __init apne_probe1(struct net_device *dev, int ioaddr)
> int neX000, ctron;
> #endif
> static unsigned version_printed;
> + struct ei_device *ei_local = netdev_priv(dev);
>
> - if (ei_debug && version_printed++ == 0)
> - printk(version);
> + if ((apne_debug & NETIF_MSG_DRV) && (version_printed++ == 0))
> + netdev_info(dev, version);
generally anything with a debug test should
likely be emitted at KERN_DEBUG level.
cheers, Joe
^ permalink raw reply
* Re: [RFC] Revert "sierra_net: keep status interrupt URB active"
From: Dan Williams @ 2013-11-08 19:29 UTC (permalink / raw)
To: Bjørn Mork; +Cc: netdev, linux-usb, John Henderson
In-Reply-To: <1383596827.29469.1.camel@dcbw.foobar.com>
On Mon, 2013-11-04 at 14:27 -0600, Dan Williams wrote:
> On Fri, 2013-11-01 at 13:53 +0100, Bjørn Mork wrote:
> > This reverts commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf.
> >
> > It's not easy to create a driver for all the various firmware
> > bugs out there.
> >
> > This change caused regressions for a number of devices, which
> > started to fail link detection and therefore became completely
> > non-functional. The exact reason is yet unknown, it looks like
> > the affected firmwares might actually need all or some of the
> > additional SYNC messages the patch got rid of.
> >
> > Reverting is not optimal, as it will re-introduce the original
> > problem, but it is currently the only alternative known to fix
> > this issue.
>
> Instead, how does the following patch work for you?
Bjorn, did you have a chance to try this patch out on your devices?
Dan
> Dan
>
> ---
> diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
> index a79e9d3..dd59d97 100644
> --- a/drivers/net/usb/sierra_net.c
> +++ b/drivers/net/usb/sierra_net.c
> @@ -163,18 +163,19 @@ struct lsi_umts {
> #define SIERRA_NET_LSI_UMTS_LEN (sizeof(struct lsi_umts))
> #define SIERRA_NET_LSI_UMTS_STATUS_LEN \
> (SIERRA_NET_LSI_UMTS_LEN - SIERRA_NET_LSI_COMMON_LEN)
>
> /* Forward definitions */
> static void sierra_sync_timer(unsigned long syncdata);
> static int sierra_net_change_mtu(struct net_device *net, int new_mtu);
> +static int sierra_net_open (struct net_device *net);
>
> /* Our own net device operations structure */
> static const struct net_device_ops sierra_net_device_ops = {
> - .ndo_open = usbnet_open,
> + .ndo_open = sierra_net_open,
> .ndo_stop = usbnet_stop,
> .ndo_start_xmit = usbnet_start_xmit,
> .ndo_tx_timeout = usbnet_tx_timeout,
> .ndo_change_mtu = sierra_net_change_mtu,
> .ndo_set_mac_address = eth_mac_addr,
> .ndo_validate_addr = eth_validate_addr,
> };
> @@ -439,14 +440,15 @@ static void sierra_net_dosync(struct usbnet *dev)
> netdev_err(dev->net,
> "Send SYNC failed, status %d\n", status);
> status = sierra_net_send_sync(dev);
> if (status < 0)
> netdev_err(dev->net,
> "Send SYNC failed, status %d\n", status);
>
> +printk(KERN_INFO "%s: sent two SYNC messages\n", __func__);
> /* Now, start a timer and make sure we get the Restart Indication */
> priv->sync_timer.function = sierra_sync_timer;
> priv->sync_timer.data = (unsigned long) dev;
> priv->sync_timer.expires = jiffies + SIERRA_NET_SYNCDELAY;
> add_timer(&priv->sync_timer);
> }
>
> @@ -497,31 +499,34 @@ static void sierra_net_kevent(struct work_struct *work)
> netdev_err(dev->net, "%s: Bad packet, received"
> " %d, expected %d\n", __func__, len,
> hh.hdrlen + hh.payload_len.word);
> kfree(buf);
> return;
> }
>
> +printk(KERN_INFO "%s: received msg 0x%02x len %d\n", __func__, hh.msgid.byte, len);
> /* Switch on received message types */
> switch (hh.msgid.byte) {
> case SIERRA_NET_HIP_LSI_UMTSID:
> dev_dbg(&dev->udev->dev, "LSI for ctx:%d",
> hh.msgspecific.byte);
> sierra_net_handle_lsi(dev, buf, &hh);
> break;
> case SIERRA_NET_HIP_RESTART_ID:
> +printk(KERN_INFO "%s: RESTART received code 0x%02x\n", __func__, hh.msgspecific.byte);
> dev_dbg(&dev->udev->dev, "Restart reported: %d,"
> " stopping sync timer",
> hh.msgspecific.byte);
> /* Got sync resp - stop timer & clear mask */
> del_timer_sync(&priv->sync_timer);
> clear_bit(SIERRA_NET_TIMER_EXPIRY,
> &priv->kevent_flags);
> break;
> case SIERRA_NET_HIP_HSYNC_ID:
> +printk(KERN_INFO "%s: HSYNC received\n", __func__);
> dev_dbg(&dev->udev->dev, "SYNC received");
> err = sierra_net_send_sync(dev);
> if (err < 0)
> netdev_err(dev->net,
> "Send SYNC failed %d\n", err);
> break;
> case SIERRA_NET_HIP_EXTENDEDID:
> @@ -537,14 +542,15 @@ static void sierra_net_kevent(struct work_struct *work)
> break;
> }
> }
> kfree(buf);
> }
> /* The sync timer bit might be set */
> if (test_bit(SIERRA_NET_TIMER_EXPIRY, &priv->kevent_flags)) {
> +printk(KERN_INFO "%s: re-sending SYNC\n", __func__);
> clear_bit(SIERRA_NET_TIMER_EXPIRY, &priv->kevent_flags);
> dev_dbg(&dev->udev->dev, "Deferred sync timer expiry");
> sierra_net_dosync(priv->usbnet);
> }
>
> if (priv->kevent_flags)
> dev_dbg(&dev->udev->dev, "sierra_net_kevent done, "
> @@ -562,14 +568,15 @@ static void sierra_net_defer_kevent(struct usbnet *dev, int work)
> /*
> * Sync Retransmit Timer Handler. On expiry, kick the work queue
> */
> static void sierra_sync_timer(unsigned long syncdata)
> {
> struct usbnet *dev = (struct usbnet *)syncdata;
>
> +printk(KERN_INFO "%s: sync timer expired\n", __func__);
> dev_dbg(&dev->udev->dev, "%s", __func__);
> /* Kick the tasklet */
> sierra_net_defer_kevent(dev, SIERRA_NET_TIMER_EXPIRY);
> }
>
> static void sierra_net_status(struct usbnet *dev, struct urb *urb)
> {
> @@ -584,14 +591,15 @@ static void sierra_net_status(struct usbnet *dev, struct urb *urb)
> event = urb->transfer_buffer;
> switch (event->bNotificationType) {
> case USB_CDC_NOTIFY_NETWORK_CONNECTION:
> case USB_CDC_NOTIFY_SPEED_CHANGE:
> /* USB 305 sends those */
> break;
> case USB_CDC_NOTIFY_RESPONSE_AVAILABLE:
> +printk(KERN_INFO "%s: firmware indicates response available\n", __func__);
> sierra_net_defer_kevent(dev, SIERRA_NET_EVENT_RESP_AVAIL);
> break;
> default:
> netdev_err(dev->net, ": unexpected notification %02x!\n",
> event->bNotificationType);
> break;
> }
> @@ -767,20 +775,32 @@ static void sierra_net_unbind(struct usbnet *dev, struct usb_interface *intf)
> /* tell modem we are going away */
> status = sierra_net_send_cmd(dev, priv->shdwn_msg,
> sizeof(priv->shdwn_msg), "Shutdown");
> if (status < 0)
> netdev_err(dev->net,
> "usb_control_msg failed, status %d\n", status);
>
> - usbnet_status_stop(dev);
> -
> sierra_net_set_private(dev, NULL);
> kfree(priv);
> }
>
> +static int sierra_net_open (struct net_device *net)
> +{
> + int ret;
> +
> + ret = usbnet_open(net);
> + if (ret == 0) {
> + struct usbnet *dev = netdev_priv(net);
> +
> + /* Interrupt URB now set up; initiate sync sequence */
> + sierra_net_dosync(dev);
> + }
> + return ret;
> +}
> +
> static struct sk_buff *sierra_net_skb_clone(struct usbnet *dev,
> struct sk_buff *skb, int len)
> {
> struct sk_buff *new_skb;
>
> /* clone skb */
> new_skb = skb_clone(skb, GFP_ATOMIC);
> @@ -910,14 +930,15 @@ static const struct driver_info sierra_net_info_direct_ip = {
> .bind = sierra_net_bind,
> .unbind = sierra_net_unbind,
> .status = sierra_net_status,
> .rx_fixup = sierra_net_rx_fixup,
> .tx_fixup = sierra_net_tx_fixup,
> };
>
> +#if 0
> static int
> sierra_net_probe(struct usb_interface *udev, const struct usb_device_id *prod)
> {
> int ret;
>
> ret = usbnet_probe(udev, prod);
> if (ret == 0) {
> @@ -927,14 +948,15 @@ sierra_net_probe(struct usb_interface *udev, const struct usb_device_id *prod)
> if (ret == 0) {
> /* Interrupt URB now set up; initiate sync sequence */
> sierra_net_dosync(dev);
> }
> }
> return ret;
> }
> +#endif
>
> #define DIRECT_IP_DEVICE(vend, prod) \
> {USB_DEVICE_INTERFACE_NUMBER(vend, prod, 7), \
> .driver_info = (unsigned long)&sierra_net_info_direct_ip}, \
> {USB_DEVICE_INTERFACE_NUMBER(vend, prod, 10), \
> .driver_info = (unsigned long)&sierra_net_info_direct_ip}, \
> {USB_DEVICE_INTERFACE_NUMBER(vend, prod, 11), \
> @@ -950,15 +972,15 @@ static const struct usb_device_id products[] = {
> };
> MODULE_DEVICE_TABLE(usb, products);
>
> /* We are based on usbnet, so let it handle the USB driver specifics */
> static struct usb_driver sierra_net_driver = {
> .name = "sierra_net",
> .id_table = products,
> - .probe = sierra_net_probe,
> + .probe = usbnet_probe,
> .disconnect = usbnet_disconnect,
> .suspend = usbnet_suspend,
> .resume = usbnet_resume,
> .no_dynamic_id = 1,
> .disable_hub_initiated_lpm = 1,
> };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 IPROUTE] ip: Add HSR support
From: Arvid Brodin @ 2013-11-08 19:34 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Stephen Hemminger, Arvid Brodin
This patch adds basic support for High-Availability Seamless
Redundancy (HSR) network devices.
Signed-off-by: Arvid Brodin <arvid.brodin@xdin.com>
---
include/linux/if_link.h | 14 ++++++
ip/Makefile | 2 +-
ip/iplink_hsr.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++
man/man8/ip-link.8.in | 1 +
4 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 ip/iplink_hsr.c
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index ee4f2ba..1f429f1 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -468,4 +468,18 @@ enum {
#define IFLA_IPOIB_MAX (__IFLA_IPOIB_MAX - 1)
+/* HSR section */
+
+enum {
+ IFLA_HSR_UNSPEC,
+ IFLA_HSR_SLAVE1,
+ IFLA_HSR_SLAVE2,
+ IFLA_HSR_MULTICAST_SPEC, /* Last byte of supervision addr */
+ IFLA_HSR_SUPERVISION_ADDR, /* Supervision frame multicast addr */
+ IFLA_HSR_SEQ_NR,
+ __IFLA_HSR_MAX,
+};
+
+#define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)
+
#endif /* _LINUX_IF_LINK_H */
diff --git a/ip/Makefile b/ip/Makefile
index f10d22f..9dad492 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -5,7 +5,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
iplink_vlan.o link_veth.o link_gre.o iplink_can.o \
iplink_macvlan.o iplink_macvtap.o ipl2tp.o link_vti.o \
iplink_vxlan.o tcp_metrics.o iplink_ipoib.o ipnetconf.o link_ip6tnl.o \
- link_iptnl.o link_gre6.o
+ link_iptnl.o link_gre6.o iplink_hsr.o
RTMONOBJ=rtmon.o
diff --git a/ip/iplink_hsr.c b/ip/iplink_hsr.c
new file mode 100644
index 0000000..4af8f78
--- /dev/null
+++ b/ip/iplink_hsr.c
@@ -0,0 +1,127 @@
+/*
+ * iplink_hsr.c HSR device support
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Arvid Brodin <arvid.brodin@xdin.com>
+ *
+ * Based on iplink_vlan.c by Patrick McHardy <kaber@trash.net>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/socket.h> /* Needed by linux/if.h for some reason */
+#include <linux/if.h>
+#include <linux/if_arp.h>
+#include "rt_names.h"
+#include "utils.h"
+#include "ip_common.h"
+
+static void usage(void)
+{
+ fprintf(stderr,
+"Usage:\tip link add name NAME type hsr slave1 SLAVE1-IF slave2 SLAVE2-IF\n"
+"\t[ supervision ADDR-BYTE ]\n"
+"\n"
+"NAME\n"
+" name of new hsr device (e.g. hsr0)\n"
+"SLAVE1-IF, SLAVE2-IF\n"
+" the two slave devices bound to the HSR device\n"
+"ADDR-BYTE\n"
+" 0-255; the last byte of the multicast address used for HSR supervision\n"
+" frames (default = 0)\n");
+}
+
+static int hsr_parse_opt(struct link_util *lu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ int ifindex;
+ unsigned char multicast_spec;
+
+ while (argc > 0) {
+ if (matches(*argv, "supervision") == 0) {
+ NEXT_ARG();
+ if (get_u8(&multicast_spec, *argv, 0))
+ invarg("ADDR-BYTE is invalid", *argv);
+ addattr_l(n, 1024, IFLA_HSR_MULTICAST_SPEC, &multicast_spec, 1);
+ } else if (matches(*argv, "slave1") == 0) {
+ NEXT_ARG();
+ ifindex = ll_name_to_index(*argv);
+ if (ifindex == 0)
+ invarg("No such interface", *argv);
+ addattr_l(n, 1024, IFLA_HSR_SLAVE1, &ifindex, 4);
+ } else if (matches(*argv, "slave2") == 0) {
+ NEXT_ARG();
+ ifindex = ll_name_to_index(*argv);
+ if (ifindex == 0)
+ invarg("No such interface", *argv);
+ addattr_l(n, 1024, IFLA_HSR_SLAVE2, &ifindex, 4);
+ } else if (matches(*argv, "help") == 0) {
+ usage();
+ return -1;
+ } else {
+ fprintf(stderr, "hsr: what is \"%s\"?\n", *argv);
+ usage();
+ return -1;
+ }
+ argc--, argv++;
+ }
+
+ return 0;
+}
+
+static void hsr_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+ SPRINT_BUF(b1);
+
+ if (!tb)
+ return;
+
+ if (tb[IFLA_HSR_SLAVE1] &&
+ RTA_PAYLOAD(tb[IFLA_HSR_SLAVE1]) < sizeof(__u32))
+ return;
+ if (tb[IFLA_HSR_SLAVE2] &&
+ RTA_PAYLOAD(tb[IFLA_HSR_SLAVE2]) < sizeof(__u32))
+ return;
+ if (tb[IFLA_HSR_SEQ_NR] &&
+ RTA_PAYLOAD(tb[IFLA_HSR_SEQ_NR]) < sizeof(__u16))
+ return;
+ if (tb[IFLA_HSR_SUPERVISION_ADDR] &&
+ RTA_PAYLOAD(tb[IFLA_HSR_SUPERVISION_ADDR]) < ETH_ALEN)
+ return;
+
+ fprintf(f, "slave1 ");
+ if (tb[IFLA_HSR_SLAVE1])
+ fprintf(f, "%s ",
+ ll_index_to_name(rta_getattr_u32(tb[IFLA_HSR_SLAVE1])));
+ else
+ fprintf(f, "<none> ");
+
+ fprintf(f, "slave2 ");
+ if (tb[IFLA_HSR_SLAVE2])
+ fprintf(f, "%s ",
+ ll_index_to_name(rta_getattr_u32(tb[IFLA_HSR_SLAVE2])));
+ else
+ fprintf(f, "<none> ");
+
+ if (tb[IFLA_HSR_SEQ_NR])
+ fprintf(f, "sequence %d ", rta_getattr_u16(tb[IFLA_HSR_SEQ_NR]));
+
+ if (tb[IFLA_HSR_SUPERVISION_ADDR])
+ fprintf(f, "supervision %s ",
+ ll_addr_n2a(RTA_DATA(tb[IFLA_HSR_SUPERVISION_ADDR]),
+ RTA_PAYLOAD(tb[IFLA_HSR_SUPERVISION_ADDR]),
+ ARPHRD_VOID,
+ b1, sizeof(b1)));
+}
+
+struct link_util hsr_link_util = {
+ .id = "hsr",
+ .maxattr = IFLA_VLAN_MAX,
+ .parse_opt = hsr_parse_opt,
+ .print_opt = hsr_print_opt,
+};
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 8b68c78..7f7f882 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -53,6 +53,7 @@ ip-link \- network device configuration
.BR bridge " | "
.BR can " | "
.BR dummy " | "
+.BR hsr " | "
.BR ifb " | "
.BR ipoib " |"
.BR macvlan " | "
--
1.8.1.5
--
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.com
^ permalink raw reply related
* Re: [patch net-next 1/2] ip6_output: fragment outgoing reassembled skb properly
From: David Miller @ 2013-11-08 19:49 UTC (permalink / raw)
To: jiri
Cc: netdev, pablo, netfilter-devel, yoshfuji, kadlec, kaber, mleitner,
kuznet, jmorris, wensong, horms, ja, edumazet, pshelar, jasowang,
alexander.h.duyck, fw
In-Reply-To: <20131108075201.GA2455@minipsycho.orion>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 8 Nov 2013 08:52:01 +0100
> Fri, Nov 08, 2013 at 12:54:53AM CET, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Wed, 6 Nov 2013 17:52:19 +0100
>>
>>> If reassembled packet would fit into outdev MTU, it is not fragmented
>>> according the original frag size and it is send as single big packet.
>>>
>>> The second case is if skb is gso. In that case fragmentation does not happen
>>> according to the original frag size.
>>>
>>> This patch fixes these.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ...
>>
>>> if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
>>> - dst_allfrag(skb_dst(skb)))
>>> + dst_allfrag(skb_dst(skb)) ||
>>> + (IP6CB(skb)->frag_max_size && skb->len > IP6CB(skb)->frag_max_size))
>>> return ip6_fragment(skb, ip6_finish_output2);
>>
>>Jiri are you sure that you don't need to take GSO into account in the
>>new part you are adding to the test?
>
>
> For gso skb, we need co cap outgoing fragments by the original frag size
> as well. So I believe that this code is correct for that case as well.
I'm still not so sure I agree, even after having taken a second look
at this.
Look at ipv4's logic for this same facility:
if (skb->len > ip_skb_dst_mtu(skb) && !skb_is_gso(skb))
return ip_fragment(skb, ip_finish_output2);
Strictly, we only call ip_fragment() if skb_is_gso() is false. And then
in ip_fragment():
if (unlikely(((iph->frag_off & htons(IP_DF)) && !skb->local_df) ||
(IPCB(skb)->frag_max_size &&
IPCB(skb)->frag_max_size > dst_mtu(&rt->dst)))) {
And that second branch of this test is what you're trying to duplicate
into ipv6.
Perhaps I don't understand completely the intentions and logic of
dst_allfrag() in the ipv6 case, and maybe you can explain it to me.
^ permalink raw reply
* Re: [PATCH] Fix "ip rule delete table 256"
From: David Miller @ 2013-11-08 19:54 UTC (permalink / raw)
To: andreas; +Cc: netdev, shemminger
In-Reply-To: <20131107172638.GA6110@amd64.fatal.se>
From: Andreas Henriksson <andreas@fatal.se>
Date: Thu, 7 Nov 2013 18:26:38 +0100
> When trying to delete a table >= 256 using iproute2 the local table
> will be deleted.
> The table id is specified as a netlink attribute when it needs more then
> 8 bits and iproute2 then sets the table field to RT_TABLE_UNSPEC (0).
> Preconditions to matching the table id in the rule delete code
> doesn't seem to take the "table id in netlink attribute" into condition
> so the frh_get_table helper function never gets to do its job when
> matching against current rule.
> Use the helper function twice instead of peaking at the table value directly.
>
> Originally reported at: http://bugs.debian.org/724783
>
> Reported-by: Nicolas HICHER <nhicher@avencall.com>
> Signed-off-by: Andreas Henriksson <andreas@fatal.se>
Good catch, applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-08 20:01 UTC (permalink / raw)
To: John Fastabend; +Cc: Anton 'EvilMan' Danilov, netdev
In-Reply-To: <1383933329.9412.243.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 2013-11-08 at 09:55 -0800, Eric Dumazet wrote:
> I do not think so : following does not work.
>
> for ETH in eth0
> do
> tc qd del dev $ETH root 2>/dev/null
>
> tc qd add dev $ETH root handle 100: mq
> for i in `seq 1 4`
> do
> tc qd add dev $ETH parent 100:$i handle $i htb
> done
> done
Humpf, I forgot a "default 1" on htb qdisc creation, sorry for the
noise ;)
^ permalink raw reply
* Re: [PATCH net] netlink: fix netlink_ack with large messages
From: David Miller @ 2013-11-08 20:07 UTC (permalink / raw)
To: jbenc; +Cc: netdev, pablo
In-Reply-To: <9333f540a9b87adbdd15e274d12a9d60994fdb34.1383850578.git.jbenc@redhat.com>
From: Jiri Benc <jbenc@redhat.com>
Date: Thu, 7 Nov 2013 19:57:45 +0100
> Commit c05cdb1b864f ("netlink: allow large data transfers from user-space")
> does not handle cases where netlink_ack is used to report an error. In such
> case, the original message is copied to the ack message, which needs to be
> large enough.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
I have two problems with this change.
First of all, if netlink_ack() has this problem, do not extend the
netlink_alloc_large_skb() usage to dumps too as your patch is
doing here.
Secondly, it seems sort of over the top to quote such enormous
messages, and in fact wasteful. We have the sequence number in
the netlink header, so the user can tell exactly which message
we are erroring.
Just quoting such huge requests in ACKs by default doesn't seem
to make any sense. I would say that we should have a way to
turn off the quoting, or at least limit it, and turn this knob
off for things like nftables that can hit these kinds of cases.
Pablo, what do you think?
^ permalink raw reply
* Re: [PATCH net-next] nfnetlink: do not ack malformed messages
From: David Miller @ 2013-11-08 20:12 UTC (permalink / raw)
To: jbenc; +Cc: netdev, pablo, pmatouse
In-Reply-To: <fa44451df8aa56773b1b17cbbd86ced8b54eca5a.1383850681.git.jbenc@redhat.com>
From: Jiri Benc <jbenc@redhat.com>
Date: Thu, 7 Nov 2013 19:59:19 +0100
> Commit 0628b123c96d ("netfilter: nfnetlink: add batch support and use it
> from nf_tables") introduced a bug leading to various crashes in netlink_ack
> when netlink message with invalid nlmsg_len was sent by an unprivileged
> user.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
This is obvious enough that I'm going to apply it directly, I hope Pablo
does not mind.
Thanks!
^ permalink raw reply
* Re: [PATCH] ipv6: use rt6_get_dflt_router to get default router in rt6_route_rcv
From: David Miller @ 2013-11-08 20:18 UTC (permalink / raw)
To: duanj.fnst; +Cc: hannes, netdev
In-Reply-To: <527C44E5.70606@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Fri, 08 Nov 2013 09:56:53 +0800
>
> As the rfc 4191 said, the Router Preference and Lifetime values in a
> ::/0 Route Information Option should override the preference and lifetime
> values in the Router Advertisement header. But when the kernel deals with
> a ::/0 Route Information Option, the rt6_get_route_info() always return
> NULL, that means that overriding will not happen, because those default
> routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router().
>
> In order to deal with that condition, we should call rt6_get_dflt_router
> when the prefix length is 0.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH v2 net-next] inet: fix a UFO regression
From: Sridhar Samudrala @ 2013-11-08 20:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexei Starovoitov, David Miller, netdev, Hannes Frederic Sowa
In-Reply-To: <1383913124.9412.223.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/8/2013 4:18 AM, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 22:41 -0800, Sridhar Samudrala wrote:
>
>> Does this fix also help vxlan performance between 2 VMs on 2 different
>> physical m/cs across 10G NIC?
>> What is the throughput you are seeing via 10G nics?
>> With linux 3.12, i am only seeing around 2Gbps (iperf TCP_STREAM with
>> 16K messages) between 2 VMs when using vxlan across a 10G nic.
>> Is this due to the overhead of software GSO and not doing GRO at the
>> receiver?
> What NIC is used at sender, what NIC is used at receiver ?
I am seeing almost similar results with both Emulex and Intel 10Gb NICs
at both sender and receiver.
>
> If you use GRE instead of VXLAN, do you get any difference in speed ?
With GRE, i am seeing slightly better results. 3.2Gb/s compared to
1.8Gb/s with vxlan.
Thanks
Sridhar
^ permalink raw reply
* Re: [net-next PATCH 0/2] ixgbe: macvlan fixes
From: David Miller @ 2013-11-08 20:21 UTC (permalink / raw)
To: john.fastabend; +Cc: jeffrey.t.kirsher, netdev, nhorman
In-Reply-To: <20131108084654.6935.36501.stgit@nitbit.x32>
From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 08 Nov 2013 00:50:07 -0800
> Two additional fixes for ixgbe l2 forwarding acceleration. One
> was reported by kbuild build bot and the second after hardening
> my test scripts.
>
> Typically I would send ixgbe fixes through JeffK's tree but I
> know a few people who are looking at the feature in their setup's
> so wanted to be sure the patches were visible.
>
> For reference the ixgbe accelerated macvlan patch is here:
>
> commit 2a47fa45d4dfbc54659d28de311a1f764b296a3c
> Author: John Fastabend <john.r.fastabend@intel.com>
> Date: Wed Nov 6 09:54:52 2013 -0800
>
> ixgbe: enable l2 forwarding acceleration for macvlans
Both applied, thanks John.
^ permalink raw reply
* kernel network panic stack traces on ubuntu 12.04
From: Templin, Fred L @ 2013-11-08 20:40 UTC (permalink / raw)
To: netdev@vger.kernel.org
In-Reply-To: <20131108.152148.475567353347933697.davem@davemloft.net>
Hi,
I am hacking the kernel network stack on a Dell running ubuntu 12.04
and when I add code that I know for sure is causing a panic the system
locks up as if it is hung and never prints out a stack trace. The only
way out is to hard reset using the power button. This happens on several
different linux kernel versions - the most recent being 3.10.17. Any
ideas how I can get a stack trace?
Thanks - Fred
^ permalink raw reply
* Re: [RFC] Revert "sierra_net: keep status interrupt URB active"
From: Bjørn Mork @ 2013-11-08 20:44 UTC (permalink / raw)
To: Dan Williams; +Cc: netdev, linux-usb, John Henderson
In-Reply-To: <1383938944.29096.7.camel@dcbw.foobar.com>
Dan Williams <dcbw@redhat.com> writes:
> On Mon, 2013-11-04 at 14:27 -0600, Dan Williams wrote:
>> On Fri, 2013-11-01 at 13:53 +0100, Bjørn Mork wrote:
>> > This reverts commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf.
>> >
>> > It's not easy to create a driver for all the various firmware
>> > bugs out there.
>> >
>> > This change caused regressions for a number of devices, which
>> > started to fail link detection and therefore became completely
>> > non-functional. The exact reason is yet unknown, it looks like
>> > the affected firmwares might actually need all or some of the
>> > additional SYNC messages the patch got rid of.
>> >
>> > Reverting is not optimal, as it will re-introduce the original
>> > problem, but it is currently the only alternative known to fix
>> > this issue.
>>
>> Instead, how does the following patch work for you?
>
> Bjorn, did you have a chance to try this patch out on your devices?
The only DirectIP device I have is the MC7710, which never had any of
the firmware issues you are trying to fix. I only tried to forward Johns
issue.
When this patch worked for John, then I am pretty confident that you
have solved the problem here.
Thanks,
Bjørn
^ permalink raw reply
* Re: [PATCH linux-2.6.32.y] sctp: unbalanced rcu lock in ip_queue_xmit()
From: Willy Tarreau @ 2013-11-08 20:48 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem, eric.dumazet, ben, stable
In-Reply-To: <1383905635-20444-1-git-send-email-nicolas.dichtel@6wind.com>
Hi Nicolas,
On Fri, Nov 08, 2013 at 11:13:55AM +0100, Nicolas Dichtel wrote:
> The bug was introduced by commit b8710128e201 ("inet: add RCU protection to
> inet->opt") (it's a backport of upstream commit f6d8bd051c39).
>
> In SCTP case, packet is already routed, hence we jump to the label
> 'packet_routed', but without rcu_read_lock(). After this label,
> rcu_read_unlock() is called unconditionally.
I've just queued it for 2.6.32.62. Thanks!
Willy
^ permalink raw reply
* Re: [PATCH v2 net-next] inet: fix a UFO regression
From: Eric Dumazet @ 2013-11-08 20:57 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: Alexei Starovoitov, David Miller, netdev, Hannes Frederic Sowa
In-Reply-To: <527D478E.4020607@gmail.com>
On Fri, 2013-11-08 at 12:20 -0800, Sridhar Samudrala wrote:
> I am seeing almost similar results with both Emulex and Intel 10Gb NICs
> at both sender and receiver.
> >
> > If you use GRE instead of VXLAN, do you get any difference in speed ?
> With GRE, i am seeing slightly better results. 3.2Gb/s compared to
> 1.8Gb/s with vxlan.
Thanks, it seems to point a bottleneck at receiver, as we have GRO
support for GRE, but not yet for VXLAN.
^ permalink raw reply
* Re: [PATCH net-next 3/8] bonding: add downdelay netlink support
From: Jay Vosburgh @ 2013-11-08 21:15 UTC (permalink / raw)
To: Scott Feldman; +Cc: Veaceslav Falico, andy, netdev, Shrijeet Mukherjee
In-Reply-To: <5C31F7A9-945E-4DA5-A2E2-92AE0A556970@cumulusnetworks.com>
Scott Feldman <sfeldma@cumulusnetworks.com> wrote:
>
>On Nov 7, 2013, at 7:05 AM, Jay Vosburgh <fubar@us.ibm.com> wrote:
>
>> Scott Feldman <sfeldma@cumulusnetworks.com> wrote:
>>
>>> +int bond_option_downdelay_set(struct bonding *bond, int downdelay)
>>> +{
>>> + if (!(bond->params.miimon)) {
>>> + pr_err("%s: Unable to set down delay as MII monitoring is disabled\n", bond->dev->name);
>>> + return -EPERM;
>>> + }
>>
>> I would argue that it is better to permit this option to be set
>> at any time, regardless of whether miimon is enabled or not. This
>> removes an ordering constraint and makes things generally easier to deal
>> with. Setting the option has no effect if miimon is not set, but that's
>> ok.
>>
>> My comment here would also apply to other options that are
>> "secondary," in the sense that they affect the behavior of other options
>> or modes, e.g., updelay, arp_validate (in another path of this series),
>> arp_ip_targets, etc.
>>
>> I'm aware that this is simply duplicating the existing behavior
>> of the sysfs API, but I'd be in favor of changing that, too. These
>> limitations are a holdover from the module paramters days, and should
>> have been made more flexible a long time ago.
>
>What I’d like to propose, and I hope that you’ll agree, is we tackle
>this in two phases. The first phase is to finish the current
>duplication effort to enable netlink equivalents of the attributes in
>sysfs. This is a fairly mechanical process, preserving existing
>behavior as you point out, and keeps the patches single-minded and easy
>to review.
>
>The second phase is to revisit ordering constraints and find places
>where we can remove constraints or streamline the dependency checks.
>
>Sound like a plan?
Well, maybe.
What I want to avoid for the iproute / netlink bonding support
is all of the hoops that the initscripts / sysconfig scripts had to jump
through to obey the current sysfs ordering limtations.
The primary user of this is going to be iproute. Will the above
quoted limitations (and their equivalents for various other options)
make any difference with regards to the following:
ip [...] bond arp_validate all arp_interval 1000
ip [...] bond arp_interval 1000 arp_validate all
I.e., does the ordering matter at the iproute level when
multiple options are specified simultaneously?
What happens if I do:
ip [...] bond arp_interval 1000 arp_validate all miimon 100
This is separate from simply changing one thing at a time, e.g.,
ip [...] bond arp_validate all
ip [...] bond arp_interval 1000
will presuambly hit the test above, and this is where the
ordering stuff comes into play for sure.
Also, as I look at the iproute patch, it doesn't appear to
accept the text names for the options, only numeric values (e.g., "ip
[...] bond arp_validate 3"). That appears to be a limitation of Jiri's
original iproute patch as well. Am I the only person that perfers the
text labels (e.g., arp_validate as "all") to numbers (arp_validate as
"3")?
That number-only limitation also may make life for initscripts /
sysconfig more difficult, or at least the upgrade path more of a hassle,
since currently the ifcfg-bond* files can have the bonding options
specified as text labels in addition to the numeric values.
So, honestly, I think if the ordering constraints are going to
be relaxed, it should happen sooner rather than later. Perhaps not in
the same patch as the netlink support, but ideally at least in the same
series, so there is no real release with the constraints. Changing the
constraints after the script, etc, conversion is done doesn't really
help much.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH net] ipv4: fix wildcard search with inet_confirm_addr()
From: Julian Anastasov @ 2013-11-08 21:24 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
In-Reply-To: <1383921428-4313-1-git-send-email-nicolas.dichtel@6wind.com>
Hello,
On Fri, 8 Nov 2013, Nicolas Dichtel wrote:
> Help of this function says: "in_dev: only on this interface, 0=any interface",
> but in fact the code supposes that it will never be NULL.
>
> Note that this function was never called with in_dev == NULL, but it's exported
> and may be used by an external module.
The initial idea was that arp_ignore=3 can provide
dev=NULL. Later argument was changed to in_dev but we should
allow the NULL value. As you are going to change the arguments
may be we can partially revert commit
39a6d06300128d32f361f4f790beba0ca83730eb, i.e. to add the
missing in_dev = NULL in arp_ignore() and also revert this change:
- if (in_dev != NULL)
+ if (scope != RT_SCOPE_LINK)
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>
> This bug was spotted by code review.
>
> drivers/net/bonding/bonding.h | 10 +++-------
> include/linux/inetdevice.h | 3 ++-
> net/ipv4/arp.c | 3 ++-
> net/ipv4/devinet.c | 8 ++++----
> 4 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 03cf3fd14490..448895e90ccb 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -407,15 +407,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
>
> static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local)
> {
> - struct in_device *in_dev;
> - __be32 addr = 0;
> + __be32 addr;
>
> rcu_read_lock();
> - in_dev = __in_dev_get_rcu(dev);
> -
> - if (in_dev)
> - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
> -
> + addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst,
> + local, RT_SCOPE_HOST);
As in_dev = NULL should be allowed, above change
will not be needed, we should keep the 'if (in_dev)' there.
> rcu_read_unlock();
> return addr;
> }
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 79640e015a86..5870f7060917 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
> extern void devinet_init(void);
> extern struct in_device *inetdev_by_index(struct net *, int);
> extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
> -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
> + __be32 local, int scope);
> extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
>
> static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7808093cede6..fc5ebc7e1962 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> default:
> return 0;
> }
> - return !inet_confirm_addr(in_dev, sip, tip, scope);
> + return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip,
> + scope);
We will need initial struct net *net = dev_net(in_dev->dev)
because later we will set in_dev to NULL for arp_ignore=3.
Then we will provide 'net' to inet_confirm_addr.
> }
>
> static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a1b5bcbd04ae..47cdb035f817 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
>
> /*
> * Confirm that local IP address exists using wildcards:
> + * - net: netns to check, cannot be NULL
> * - in_dev: only on this interface, 0=any interface
NULL is better than 0
> * - dst: only in the same subnet as dst, 0=any dst
> * - local: address, 0=autoselect the local address
> * - scope: maximum allowed scope value for the local address
> */
> -__be32 inet_confirm_addr(struct in_device *in_dev,
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
> __be32 dst, __be32 local, int scope)
> {
> __be32 addr = 0;
> struct net_device *dev;
> - struct net *net;
>
> if (scope != RT_SCOPE_LINK)
Use if (in_dev)
> - return confirm_addr_indev(in_dev, dst, local, scope);
> + return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) :
> + 0;
We will not need the above change.
> - net = dev_net(in_dev->dev);
> rcu_read_lock();
> for_each_netdev_rcu(net, dev) {
> in_dev = __in_dev_get_rcu(dev);
> --
> 1.8.4.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [RFC] Revert "sierra_net: keep status interrupt URB active"
From: Dan Williams @ 2013-11-08 21:29 UTC (permalink / raw)
To: Bjørn Mork
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
John Henderson
In-Reply-To: <87txfm8vz5.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Fri, 2013-11-08 at 21:44 +0100, Bjørn Mork wrote:
> Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> > On Mon, 2013-11-04 at 14:27 -0600, Dan Williams wrote:
> >> On Fri, 2013-11-01 at 13:53 +0100, Bjørn Mork wrote:
> >> > This reverts commit 7b0c5f21f348a66de495868b8df0284e8dfd6bbf.
> >> >
> >> > It's not easy to create a driver for all the various firmware
> >> > bugs out there.
> >> >
> >> > This change caused regressions for a number of devices, which
> >> > started to fail link detection and therefore became completely
> >> > non-functional. The exact reason is yet unknown, it looks like
> >> > the affected firmwares might actually need all or some of the
> >> > additional SYNC messages the patch got rid of.
> >> >
> >> > Reverting is not optimal, as it will re-introduce the original
> >> > problem, but it is currently the only alternative known to fix
> >> > this issue.
> >>
> >> Instead, how does the following patch work for you?
> >
> > Bjorn, did you have a chance to try this patch out on your devices?
>
> The only DirectIP device I have is the MC7710, which never had any of
> the firmware issues you are trying to fix. I only tried to forward Johns
> issue.
>
> When this patch worked for John, then I am pretty confident that you
> have solved the problem here.
Well, "solved", since I still have no idea why the original patch would
cause the device behavior based on what I know and have read about the
expected firmware/host handshaking sequence. But the patch there
appears to fix the problem *and* not blindly send tons of SYNCs forever.
So I'll go ahead and submit a proper version of it.
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: iproute2: potential upgrade regression with 58a3e827
From: Eric W. Biederman @ 2013-11-08 21:36 UTC (permalink / raw)
To: Chris J Arges; +Cc: shemminger, netdev
In-Reply-To: <527D2768.1030403@canonical.com>
Chris J Arges <chris.j.arges@canonical.com> writes:
> Hi,
> The commit
> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
> could be potentially introducing a regression on an upgrade.
>
> I've noticed that upgrading iproute while there are active namespaces
> could cause the following error:
> seting the network namespace failed: Invalid argument
>
> Here's a test case:
> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>
> In one terminal run:
> ip netns add netns_old
> ip link add name if_old type veth peer name if_old_peer
> ip link set dev if_old_peer netns netns_old
> ip netns exec netns_old bash
>
> Build and install iproute2 with 58a3e8270fe72f8ed92687d3a3132c2a708582dd
>
> In the same terminal as you typed the original commands run:
> ip netns add netns_one
> ip link add name if_one type veth peer name if_one_peer
> ip link set dev if_one_peer netns netns_one
> ip netns exec netns_one bash
> ip netns exec netns_old bash
>
> You'll get:
> seting the network namespace failed: Invalid argument
>
> If you just run the above without transitioning to the code in 58a3e827,
> then it works.
ip netns exec is fundamentally designed to run code that knows nothing
about network namespaces without modification. If we can make network
namespace aware code work that is great but the entire point of ip netns
exec is to remove the need to convert every network aware program in
linux to be network namespace aware.
Shared subtree support that is added in 58a3e827 should make things a
little better, by letting changes that happen in the initial mount
namespace propogate into children running inside of ip netns exec.
It appears that somehow you wound up in a mount namespace where the
mount of netns_one did not propagate too. Which is weird. I expect
we are missing an error message somewhere.
So far nothing allows network namespaces added like:
ip netns exec ns1 ip netns add ns2
to propogate up to the initial mount namespace and that will definitely
cause problems, but not the problem you are seeing.
Shrug. I will be happy to review patches to make this better but I
don't care enough to investigate. If you aren't up to writing patches
to make ip netns work inside of ip netns exec I recommend you don't do
that then.
Eric
^ permalink raw reply
* Re: Using HTB over MultiQ
From: Eric Dumazet @ 2013-11-08 21:56 UTC (permalink / raw)
To: Anton 'EvilMan' Danilov; +Cc: John Fastabend, netdev
In-Reply-To: <CAEzD07LmgvvfNhNuxzwjy7fxLsCBLiofeXJ61qbdOBQCiSRefw@mail.gmail.com>
On Fri, 2013-11-08 at 19:53 +0400, Anton 'EvilMan' Danilov wrote:
> Hm.. I think I need another way to classify packets for using of mq
> qdisc, because it don't support classify at root (not supported error
> on try attach filter).
I think it might be time to add filters on the device, and run them
without any lock (rcu protection only)
John, I think you had some previous work ?
> On next week i'll try to write ipset extension for setting of priority
> and queue_mapping value through the IPSET netfilter target.
Sounds a nice idea as well.
^ permalink raw reply
* [PATCH] libertas_usb: fix leak at failure path in if_usb_probe()
From: Alexey Khoroshilov @ 2013-11-08 21:57 UTC (permalink / raw)
To: John W. Linville
Cc: Alexey Khoroshilov, libertas-dev, linux-wireless, netdev,
linux-kernel, ldv-project
Error handling code in if_usb_probe() misses usb_put_dev(udev).
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/wireless/libertas/if_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
index 2798077..7aec7b2 100644
--- a/drivers/net/wireless/libertas/if_usb.c
+++ b/drivers/net/wireless/libertas/if_usb.c
@@ -285,6 +285,7 @@ static int if_usb_probe(struct usb_interface *intf,
return 0;
err_get_fw:
+ usb_put_dev(udev);
lbs_remove_card(priv);
err_add_card:
if_usb_reset_device(cardp);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH net] net: sch_red: Fix race between timer and red_destroy()
From: Vijay Subramanian @ 2013-11-08 21:59 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Stephen Hemminger, Eric Dumazet
In-Reply-To: <20131105.221633.636594084725891709.davem@davemloft.net>
> del_timer_sync() waits for all instances of the timer handler which
> are running to complete, and then it deletes the timer from any of
> the scheduled timer lists.
>
> Your patch is making the mod_timer() in the timer handler conditional,
> and this really shouldn't be necessary.
>
> The whole point of del_timer_sync() is to address these kinds of
> situations.
Thanks for the explanation Dave. I am trying to reconcile your
explanation with the text above del_timer_sync()
vim +1013 kernel/timer.c
" * This function only differs from del_timer() on SMP: besides deactivating
* the timer it also makes sure the handler has finished executing on other
* CPUs.
*
* Synchronization rules: Callers must prevent restarting of the timer,
* otherwise this function is meaningless. ..."
This seems to suggest simply calling del_timer_sync() is not enough.
In fact, for the same reason I was recently pointed towards
commit 980c478ddbb72 (sch_sfq: use del_timer_sync() in sfq_destroy() )
and I based the patch on how q->perturb_period is reset in
sfq_destroy() and used in sfq_perturbation().
Based on your explanation , I think we do not need (in sched/sch_sfq.c)
q->perturb_period = 0; in sfq_destroy() and just del_timer_sync()
should be enough.
Thanks,
Vijay
^ permalink raw reply
* Re: [patch net-next 1/2] ip6_output: fragment outgoing reassembled skb properly
From: Hannes Frederic Sowa @ 2013-11-08 22:20 UTC (permalink / raw)
To: David Miller
Cc: jiri, netdev, pablo, netfilter-devel, yoshfuji, kadlec, kaber,
mleitner, kuznet, jmorris, wensong, horms, ja, edumazet, pshelar,
jasowang, alexander.h.duyck, fw
In-Reply-To: <20131108.144915.635810300359291522.davem@davemloft.net>
On Fri, Nov 08, 2013 at 02:49:15PM -0500, David Miller wrote:
> Perhaps I don't understand completely the intentions and logic of
> dst_allfrag() in the ipv6 case, and maybe you can explain it to me.
dst_allfrag gets active if we receive a PMTU packet indicating a MTU
smaller than 1280. Of course IPv6 may not go below 1280 but this indicates
e.g. a IPv6->IPv4 migration technology is on the path which needs the
IPv6 fragment header and its fragment id to generate IPv4 fragment ids
out of the IPv6 ones to produce IPv4 fragments just after this migration
router which could well be smaller than 1280.
Maybe this does help a bit,
Hannes
^ permalink raw reply
* Re: iproute2: potential upgrade regression with 58a3e827
From: Chris J Arges @ 2013-11-08 22:30 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: shemminger, netdev
In-Reply-To: <87k3gi4lub.fsf@xmission.com>
On 11/08/2013 03:36 PM, Eric W. Biederman wrote:
> Chris J Arges <chris.j.arges@canonical.com> writes:
>
>> Hi,
>> The commit
>> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
>> could be potentially introducing a regression on an upgrade.
>>
>> I've noticed that upgrading iproute while there are active namespaces
>> could cause the following error:
>> seting the network namespace failed: Invalid argument
>>
>> Here's a test case:
>> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>>
>> In one terminal run:
>> ip netns add netns_old
>> ip link add name if_old type veth peer name if_old_peer
>> ip link set dev if_old_peer netns netns_old
>> ip netns exec netns_old bash
>>
>> Build and install iproute2 with 58a3e8270fe72f8ed92687d3a3132c2a708582dd
>>
>> In the same terminal as you typed the original commands run:
>> ip netns add netns_one
>> ip link add name if_one type veth peer name if_one_peer
>> ip link set dev if_one_peer netns netns_one
>> ip netns exec netns_one bash
>> ip netns exec netns_old bash
>>
>> You'll get:
>> seting the network namespace failed: Invalid argument
>>
>> If you just run the above without transitioning to the code in 58a3e827,
>> then it works.
>
> ip netns exec is fundamentally designed to run code that knows nothing
> about network namespaces without modification. If we can make network
> namespace aware code work that is great but the entire point of ip netns
> exec is to remove the need to convert every network aware program in
> linux to be network namespace aware.
>
> Shared subtree support that is added in 58a3e827 should make things a
> little better, by letting changes that happen in the initial mount
> namespace propogate into children running inside of ip netns exec.
>
> It appears that somehow you wound up in a mount namespace where the
> mount of netns_one did not propagate too. Which is weird. I expect
> we are missing an error message somewhere.
>
> So far nothing allows network namespaces added like:
> ip netns exec ns1 ip netns add ns2
> to propogate up to the initial mount namespace and that will definitely
> cause problems, but not the problem you are seeing.
>
> Shrug. I will be happy to review patches to make this better but I
> don't care enough to investigate. If you aren't up to writing patches
> to make ip netns work inside of ip netns exec I recommend you don't do
> that then.
>
> Eric
Fair enough. I really wanted to get a feel if this seemed like an actual
bug, or rather it's behaving like we'd expect. I don't mind writing
code, but I want to know if I'm hunting a bug, or adding a feature.
Thanks for the quick reply,
--chris j arges
^ permalink raw reply
* Re: iproute2: potential upgrade regression with 58a3e827
From: Eric W. Biederman @ 2013-11-08 22:42 UTC (permalink / raw)
To: Chris J Arges; +Cc: shemminger, netdev
In-Reply-To: <527D65FB.8000407@canonical.com>
Chris J Arges <chris.j.arges@canonical.com> writes:
> Fair enough. I really wanted to get a feel if this seemed like an actual
> bug, or rather it's behaving like we'd expect. I don't mind writing
> code, but I want to know if I'm hunting a bug, or adding a feature.
Making it ip netns add work reliably inside of ip netns exec and
handling all of the permutations is adding a feature.
Understanding why ip netns exec is failing in your specific case sounds
like there may be a missing error message. Clearly you get into a
context where /var/run/netns/netns_old is no the bind mount we would
expect it to be.
I saw nothing that should unmake /var/run/netns/nnetns_old as a mount
point. So something strange is going on.
So you might want to go through and inspect to see what is happening.
Perhaps there is a missing error message somewhere.
Without understanding why that mountpoint fails to exist I can't say if
it is a real bug or if it just something weird caused by using an ip
netns exec it was not designed to be used and is known not to be 100%
robust in.
Eric
^ 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