* Re: [PATCH net-next] net: mark some potential candidates __read_mostly
From: David Miller @ 2015-01-31 1:58 UTC (permalink / raw)
To: dborkman; +Cc: netdev
In-Reply-To: <1422530103-23869-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 29 Jan 2015 12:15:03 +0100
> They are all either written once or extremly rarely (e.g. from init
> code), so we can move them to the .data..read_mostly section.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix passing wrong parameter header to param_type2af in sctp_process_param
From: David Miller @ 2015-01-31 1:45 UTC (permalink / raw)
To: dborkman; +Cc: vyasevich, netdev, linux-sctp, saran.neti
In-Reply-To: <1422525958-5737-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 29 Jan 2015 11:05:58 +0100
> From: Saran Maruti Ramanara <saran.neti@telus.com>
>
> When making use of RFC5061, section 4.2.4. for setting the primary IP
> address, we're passing a wrong parameter header to param_type2af(),
> resulting always in NULL being returned.
>
> At this point, param.p points to a sctp_addip_param struct, containing
> a sctp_paramhdr (type = 0xc004, length = var), and crr_id as a correlation
> id. Followed by that, as also presented in RFC5061 section 4.2.4., comes
> the actual sctp_addr_param, which also contains a sctp_paramhdr, but
> this time with the correct type SCTP_PARAM_IPV{4,6}_ADDRESS that
> param_type2af() can make use of. Since we already hold a pointer to
> addr_param from previous line, just reuse it for param_type2af().
>
> Fixes: d6de3097592b ("[SCTP]: Add the handling of "Set Primary IP Address" parameter to INIT")
> Signed-off-by: Saran Maruti Ramanara <saran.neti@telus.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] netlink: fix wrong subscription bitmask to group mapping in
From: David Miller @ 2015-01-31 1:44 UTC (permalink / raw)
To: pablo; +Cc: netdev, colona, andre
In-Reply-To: <1422525113-5698-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 29 Jan 2015 10:51:53 +0100
> The subscription bitmask passed via struct sockaddr_nl is converted to
> the group number when calling the netlink_bind() and netlink_unbind()
> callbacks.
>
> The conversion is however incorrect since bitmask (1 << 0) needs to be
> mapped to group number 1. Note that you cannot specify the group number 0
> (usually known as _NONE) from setsockopt() using NETLINK_ADD_MEMBERSHIP
> since this is rejected through -EINVAL.
>
> This problem became noticeable since 97840cb ("netfilter: nfnetlink:
> fix insufficient validation in nfnetlink_bind") when binding to bitmask
> (1 << 0) in ctnetlink.
>
> Reported-by: Andre Tomt <andre@tomt.net>
> Reported-by: Ivan Delalande <colona@arista.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied, thanks Pablo.
^ permalink raw reply
* Re: [PATCH 0/3] Changes to cpsw and davinci_emac for getting MAC address
From: David Miller @ 2015-01-31 1:42 UTC (permalink / raw)
To: tony; +Cc: netdev, linux-omap
In-Reply-To: <1422473586-18100-1-git-send-email-tony@atomide.com>
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 28 Jan 2015 11:33:03 -0800
> Here are a few patches to add common code for cpsw and davinci_emac for
> getting the MAC address. Looks like we can also now add code to get the
> MAC address on 3517 but in a slightly different way.
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] net: hip04: add missing MODULE_LICENSE
From: Ding Tianhong @ 2015-01-31 1:28 UTC (permalink / raw)
To: Arnd Bergmann, netdev; +Cc: David Miller, Zhangfei Gao
In-Reply-To: <4018099.9I7f6kd5jC@wuerfel>
On 2015/1/31 5:57, Arnd Bergmann wrote:
> The hip04 ethernet driver causes a new compile-time warning
> when built as a loadable module:
>
> WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/hisilicon/hip04_eth.o
> see include/linux/module.h for more information
>
> This adds the license as "GPL", which matches the header of the file.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
> index c02b81bcfffb..b72d238695d7 100644
> --- a/drivers/net/ethernet/hisilicon/hip04_eth.c
> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
> @@ -968,3 +968,4 @@ static struct platform_driver hip04_mac_driver = {
> module_platform_driver(hip04_mac_driver);
>
> MODULE_DESCRIPTION("HISILICON P04 Ethernet driver");
> +MODULE_LICENSE("GPL");
>
>
Good catch.
Acked-by: Ding Tianhong <dingtianhong@huawei.com>
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/6] net: Add STT tunneling protocol.
From: Tom Herbert @ 2015-01-31 1:31 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, Linux Netdev List, Jesse Gross
In-Reply-To: <CALnjE+okwQ0FBzq5YVSKDgkdo=u=DhCkX-Voo6qeSwOMQD-S0w@mail.gmail.com>
On Thu, Jan 29, 2015 at 9:02 PM, Pravin Shelar <pshelar@nicira.com> wrote:
> On Thu, Jan 29, 2015 at 8:10 PM, Tom Herbert <therbert@google.com> wrote:
>> On Thu, Jan 29, 2015 at 3:29 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>>> This adds a device level support for Stateless TCP Tunnel (STT)
>>> protocol encapsulation. NF-hook is used for receiving STT
>>> packets from networking stack.
>>> Open vSwitch can be used for configuring, set up and tear down
>>> STT tunnels.
>>>
>>> The protocol is documented at
>>> http://www.ietf.org/archive/id/draft-davie-stt-06.txt
>>>
>>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>>> Signed-off-by: Jesse Gross <jesse@nicira.com>
>>> ---
>>> include/net/stt.h | 59 +++
>>> net/ipv4/Kconfig | 11 +
>>> net/ipv4/Makefile | 1 +
>>> net/ipv4/stt.c | 1398 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 1469 insertions(+)
>>> create mode 100644 include/net/stt.h
>>> create mode 100644 net/ipv4/stt.c
>>>
>
> ....
>>> +
>>> +static void tcp_sock_release(struct socket *sock)
>>> +{
>>> + kernel_sock_shutdown(sock, SHUT_RDWR);
>>> + sk_release_kernel(sock->sk);
>>> +}
>>> +
>>> +static int tcp_sock_create4(struct net *net, __be16 port,
>>> + struct socket **sockp)
>>> +{
>>> + int err;
>>> + struct socket *sock = NULL;
>>> + struct sockaddr_in tcp_addr;
>>> +
>>> + err = sock_create_kern(AF_INET, SOCK_STREAM, 0, &sock);
>>
>> Should be: err = sock_create_kern(AF_INET, SOCK_STREAM, IPPROTO_TCP, &sock);
>>
>
> ok.
>
>>> + if (err < 0)
>>> + goto error;
>>> +
>>> + sk_change_net(sock->sk, net);
>>> +
>>> + memset(&tcp_addr, 0, sizeof(tcp_addr));
>>> + tcp_addr.sin_family = AF_INET;
>>> + tcp_addr.sin_addr.s_addr = htonl(INADDR_ANY);
>>> + tcp_addr.sin_port = port;
>>> + err = kernel_bind(sock, (struct sockaddr *)&tcp_addr,
>>> + sizeof(tcp_addr));
>>
>> I think there should also be a kernel_listen also. I'm a little
>> surprised that this could work without it, it doesn't seem like we
>> should be finding closed sockets on input. In any case even if that
>> did work, it is quite annoying that the socket doesn't appear in
>> "netstat -ant".
>>
>
> This is fake TCP socket. it is created to gain exclusive access to the
> TCP port. STT receives packet from nf-hook, so no need to listening
> TCP socket.
> I am not updating any socket stats so I do not think you will see any
> socket stats.
>
Neither the system nor the sysadmin will know these are "fake" TCP
sockets. To the outside world this looks like TCP packets being sent
to a closed port with no RSTs nor replies being generated (one
conclusion may be DOS attack of some sort). These sockets need to be
visible in management tools. If you call listen on it at least we'll
see this in the open connections list, but it would be better to
create a completely new TCP state for STT sockets and display that.
>
>>> + if (err < 0)
>>> + goto error;
>>> +
>>> + *sockp = sock;
>>> + return 0;
^ permalink raw reply
* Re: [PATCH net v2] net: ipv6: allow explicitly choosing optimistic addresses
From: David Miller @ 2015-01-31 1:31 UTC (permalink / raw)
To: ek; +Cc: netdev, hannes
In-Reply-To: <1422446619-9502-1-git-send-email-ek@google.com>
Erik, you have to address and respond to the feedback you received
from Lorenzo Colitti.
If you do not respond and handle that soon, I'm simply taking
your patch out of my queue and you'll have to resubmit even
if no changes are necessary.
Thanks.
^ permalink raw reply
* [PATCH net-next] cxgb4 : Improve IEEE DCBx support, other minor open-lldp fixes
From: Anish Bhatt @ 2015-01-31 1:20 UTC (permalink / raw)
To: netdev; +Cc: davem, hariprasad, Anish Bhatt
* Add support for IEEE ets & pfc api.
* Fix bug that resulted in incorrect bandwidth percentage being returned for
CEE peers
* Convert pfc enabled info from firmware format to what dcbnl expects before
returning
Signed-off-by: Anish Bhatt <anish@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 98 +++++++++++++++++++++++++-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h | 11 +++
2 files changed, 107 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index a35d1ec6950e..b65a5bda3195 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -428,7 +428,10 @@ static void cxgb4_getpgtccfg(struct net_device *dev, int tc,
}
*pgid = (be32_to_cpu(pcmd.u.dcb.pgid.pgid) >> (tc * 4)) & 0xf;
- INIT_PORT_DCB_READ_PEER_CMD(pcmd, pi->port_id);
+ if (local)
+ INIT_PORT_DCB_READ_LOCAL_CMD(pcmd, pi->port_id);
+ else
+ INIT_PORT_DCB_READ_PEER_CMD(pcmd, pi->port_id);
pcmd.u.dcb.pgrate.type = FW_PORT_DCB_TYPE_PGRATE;
err = t4_wr_mbox(adap, adap->mbox, &pcmd, sizeof(pcmd), &pcmd);
if (err != FW_PORT_DCB_CFG_SUCCESS) {
@@ -900,6 +903,88 @@ cxgb4_ieee_negotiation_complete(struct net_device *dev,
(dcb->supported & DCB_CAP_DCBX_VER_IEEE));
}
+static int cxgb4_ieee_read_ets(struct net_device *dev, struct ieee_ets *ets,
+ int local)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct port_dcb_info *dcb = &pi->dcb;
+ struct adapter *adap = pi->adapter;
+ uint32_t tc_info;
+ struct fw_port_cmd pcmd;
+ int i, bwg, err;
+
+ if (!(dcb->msgs & (CXGB4_DCB_FW_PGID | CXGB4_DCB_FW_PGRATE)))
+ return 0;
+
+ ets->ets_cap = dcb->pg_num_tcs_supported;
+
+ if (local) {
+ ets->willing = 1;
+ INIT_PORT_DCB_READ_LOCAL_CMD(pcmd, pi->port_id);
+ } else {
+ INIT_PORT_DCB_READ_PEER_CMD(pcmd, pi->port_id);
+ }
+
+ pcmd.u.dcb.pgid.type = FW_PORT_DCB_TYPE_PGID;
+ err = t4_wr_mbox(adap, adap->mbox, &pcmd, sizeof(pcmd), &pcmd);
+ if (err != FW_PORT_DCB_CFG_SUCCESS) {
+ dev_err(adap->pdev_dev, "DCB read PGID failed with %d\n", -err);
+ return err;
+ }
+
+ tc_info = be32_to_cpu(pcmd.u.dcb.pgid.pgid);
+
+ if (local)
+ INIT_PORT_DCB_READ_LOCAL_CMD(pcmd, pi->port_id);
+ else
+ INIT_PORT_DCB_READ_PEER_CMD(pcmd, pi->port_id);
+
+ pcmd.u.dcb.pgrate.type = FW_PORT_DCB_TYPE_PGRATE;
+ err = t4_wr_mbox(adap, adap->mbox, &pcmd, sizeof(pcmd), &pcmd);
+ if (err != FW_PORT_DCB_CFG_SUCCESS) {
+ dev_err(adap->pdev_dev, "DCB read PGRATE failed with %d\n",
+ -err);
+ return err;
+ }
+
+ for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) {
+ bwg = (tc_info >> ((7 - i) * 4)) & 0xF;
+ ets->prio_tc[i] = bwg;
+ ets->tc_tx_bw[i] = pcmd.u.dcb.pgrate.pgrate[i];
+ ets->tc_rx_bw[i] = ets->tc_tx_bw[i];
+ ets->tc_tsa[i] = pcmd.u.dcb.pgrate.tsa[i];
+ }
+
+ return 0;
+}
+
+static int cxgb4_ieee_get_ets(struct net_device *dev, struct ieee_ets *ets)
+{
+ return cxgb4_ieee_read_ets(dev, ets, 1);
+}
+
+/* We reuse this for peer PFC as well, as we can't have it enabled one way */
+static int cxgb4_ieee_get_pfc(struct net_device *dev, struct ieee_pfc *pfc)
+{
+ struct port_info *pi = netdev2pinfo(dev);
+ struct port_dcb_info *dcb = &pi->dcb;
+
+ memset(pfc, 0, sizeof(struct ieee_pfc));
+
+ if (!(dcb->msgs & CXGB4_DCB_FW_PFC))
+ return 0;
+
+ pfc->pfc_cap = dcb->pfc_num_tcs_supported;
+ pfc->pfc_en = bitswap_1(dcb->pfcen);
+
+ return 0;
+}
+
+static int cxgb4_ieee_peer_ets(struct net_device *dev, struct ieee_ets *ets)
+{
+ return cxgb4_ieee_read_ets(dev, ets, 0);
+}
+
/* Fill in the Application User Priority Map associated with the
* specified Application.
* Priority for IEEE dcb_app is an integer, with 0 being a valid value
@@ -1106,14 +1191,23 @@ static int cxgb4_cee_peer_getpfc(struct net_device *dev, struct cee_pfc *pfc)
struct port_info *pi = netdev2pinfo(dev);
cxgb4_getnumtcs(dev, DCB_NUMTCS_ATTR_PFC, &(pfc->tcs_supported));
- pfc->pfc_en = pi->dcb.pfcen;
+
+ /* Firmware sends this to us in a formwat that is a bit flipped version
+ * of spec, correct it before we send it to host. This is taken care of
+ * by bit shifting in other uses of pfcen
+ */
+ pfc->pfc_en = bitswap_1(pi->dcb.pfcen);
return 0;
}
const struct dcbnl_rtnl_ops cxgb4_dcb_ops = {
+ .ieee_getets = cxgb4_ieee_get_ets,
+ .ieee_getpfc = cxgb4_ieee_get_pfc,
.ieee_getapp = cxgb4_ieee_getapp,
.ieee_setapp = cxgb4_ieee_setapp,
+ .ieee_peer_getets = cxgb4_ieee_peer_ets,
+ .ieee_peer_getpfc = cxgb4_ieee_get_pfc,
/* CEE std */
.getstate = cxgb4_getstate,
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
index 31ce425616c9..ccf24d3dc982 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h
@@ -136,6 +136,17 @@ void cxgb4_dcb_handle_fw_update(struct adapter *, const struct fw_port_cmd *);
void cxgb4_dcb_set_caps(struct adapter *, const struct fw_port_cmd *);
extern const struct dcbnl_rtnl_ops cxgb4_dcb_ops;
+static inline __u8 bitswap_1(unsigned char val)
+{
+ return ((val & 0x80) >> 7) |
+ ((val & 0x40) >> 5) |
+ ((val & 0x20) >> 3) |
+ ((val & 0x10) >> 1) |
+ ((val & 0x08) << 1) |
+ ((val & 0x04) << 3) |
+ ((val & 0x02) << 5) |
+ ((val & 0x01) << 7);
+}
#define CXGB4_DCB_ENABLED true
#else /* !CONFIG_CHELSIO_T4_DCB */
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net] qlge: Fix qlge_update_hw_vlan_features to handle if interface is down
From: Harish Patil @ 2015-01-30 23:40 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, netdev; +Cc: cdupuis@redhat.com
In-Reply-To: <10e24de024f81e60ab64a6f0bf9647d27eb40be1.1422618157.git.mleitner@redhat.com>
>
>Currently qlge_update_hw_vlan_features() will always first put the
>interface down, then update features and then bring it up again. But it
>is possible to hit this code while the adapter is down and this causes a
>non-paired call to napi_disable(), which will get stuck.
>
>This patch fixes it by skipping these down/up actions if the interface
>is already down.
>
>Fixes: a45adbe8d352 ("qlge: Enhance nested VLAN (Q-in-Q) handling.")
>Cc: Harish Patil <harish.patil@qlogic.com>
>Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
>---
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 26
>++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
Acked-by: Harish Patil <harish.patil@qlogic.com>
Thanks,
Harish
________________________________
This message and any attached documents contain information from the sending company or its parent company(s), subsidiaries, divisions or branch offices that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
^ permalink raw reply
* Re: [PATCH v2 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Rasmus Villemoes @ 2015-01-30 23:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1422613655.31903.351.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Fri, Jan 30 2015, Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, 2015-01-29 at 15:29 +0100, Rasmus Villemoes wrote:
>> On Thu, Jan 29 2015, Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>
>> >> *
>> >> * Return:
>> >> - * The amount of the characters processed to the destination buffer, or
>> >> - * %-ENOMEM if the size of buffer is not enough to put an escaped character is
>> >> - * returned.
>> >> - *
>> >> - * Even in the case of error @dst pointer will be updated to point to the byte
>> >> - * after the last processed character.
>> >> + * The total size of the escaped output that would be generated for
>> >> + * the given input and flags. To check whether the output was
>> >> + * truncated, compare the return value to osz. There is room left in
>> >> + * dst for a '\0' terminator if and only if ret < osz.
>> >> */
>> >> -int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
>> >> - unsigned int flags, const char *esc)
>> >> +size_t string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
>> >> + unsigned int flags, const char *esc)
>> >
>> > I prefer to leave the prototype the same. int for return is okay. dst
>> > should be updated accordingly.
>>
>> Please explain exactly what you think the return value should be, and
>> what *dst should be set to.
>
> Return value like you proposed, *dst is incremented by it.
The more I think about it, the less I like having dst being char**.
(1) It is unlike most other functions taking buffer+size arguments.
(2) It is inconvenient. Callers doing
char escaped[SOME_SIZE];
or
char *escaped = kmalloc(likely_big_enough);
can't just pass &escaped; they would need to use an extra dummy variable.
(3) With the return value telling the size of the would-be generated
output, it is redundant.
In fact, I dislike it so much that I'm not going to sign off on a patch
where dst is still char**.
>> > Could it be a part of nomem test still?
>>
>> What nomem test? string_escape_mem with snprintf-like semantics cannot
>> return an error; that has to be checked by the caller.
>
> Make this code a separate test, which actually still nomem, since you
> have not enough memory in the destination buffer. What the problem to
> check for proper return value and the last couple of characters written
> to the destination buffer?
Sure, it could go into a separate helper. Anyway, the semantics of
string_escape_mem needs to be decided before it makes sense to update
the tests.
> When your series will be ready (and actually I recommend to push first
> patch apart from the rest since it's not related)
I agree on that part. Andrew, could you take 1/3
(http://thread.gmane.org/gmane.linux.kernel/1876841/focus=348404), and
then we'll see when the %pE case gets sorted out.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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: [PATCH] net: rocker: Add support for retrieving port level statistics
From: Scott Feldman @ 2015-01-30 22:50 UTC (permalink / raw)
To: David Ahern; +Cc: Netdev
In-Reply-To: <1422590373-22807-1-git-send-email-dsahern@gmail.com>
On Thu, Jan 29, 2015 at 7:59 PM, David Ahern <dsahern@gmail.com> wrote:
> Add support for retrieving port level statistics from device.
> Hook is added for ethtool's stats functionality. For example,
>
> $ ethtool -S eth3
> NIC statistics:
> rx_packets: 12
> rx_bytes: 2790
> rx_dropped: 0
> rx_errors: 0
> tx_packets: 8
> tx_bytes: 728
> tx_dropped: 0
> tx_errors: 0
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Acked-by: Scott Feldman <sfeldma@gmail.com>
^ permalink raw reply
* RE: [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode
From: Skidmore, Donald C @ 2015-01-30 22:13 UTC (permalink / raw)
To: Hiroshi Shimamoto, Alexander Duyck, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E1908D@BPXM14GP.gisp.nec.co.jp>
> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Friday, January 30, 2015 3:37 AM
> To: Alexander Duyck; Skidmore, Donald C; Bjørn Mork
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> Jong; linux-kernel@vger.kernel.org; David Laight; Hayato Momma
> Subject: [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to enable MC
> promiscuous mode
>
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> The limitation of the number of multicast address for VF is not enough for
> the large scale server with SR-IOV feature.
> IPv6 requires the multicast MAC address for each IP address to handle the
> Neighbor Solicitation message.
> We couldn't assign over 30 IPv6 addresses to a single VF interface.
>
> The easy way to solve this is enabling multicast promiscuous mode.
> It is good to have a functionality to enable multicast promiscuous mode for
> each VF from VF driver.
>
> This patch introduces the new mbox API, IXGBE_VF_SET_MC_PROMISC, to
> enable/disable multicast promiscuous mode in VF. If multicast promiscuous
> mode is enabled the VF can receive all multicast packets.
>
> With this patch, the ixgbevf driver automatically enable multicast
> promiscuous mode when the number of multicast addresses is over than 30
> if possible.
>
> This also bump the API version up to 1.2 to check whether the API,
> IXGBE_VF_SET_MC_PROMISC is available.
>
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
> CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 4 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 89
> ++++++++++++++++++++++-
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 13 +++-
> drivers/net/ethernet/intel/ixgbevf/mbx.h | 4 +
> drivers/net/ethernet/intel/ixgbevf/vf.c | 29 +++++++-
> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 +
> 7 files changed, 137 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index b6137be..bf3333e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -144,6 +144,7 @@ struct vf_data_storage {
> u16 vlans_enabled;
> bool clear_to_send;
> bool pf_set_mac;
> + bool mc_promisc;
> u16 pf_vlan; /* When set, guest VLAN config not allowed. */
> u16 pf_qos;
> u16 tx_rate;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> index a5cb755..2963557 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> @@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
> /* This value should always be last */
> ixgbe_mbox_api_unknown, /* indicates that API version is not
> known */
> };
> @@ -91,6 +92,9 @@ enum ixgbe_pfvf_api_rev {
> /* mailbox API, version 1.1 VF requests */
> #define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
>
> +/* mailbox API, version 1.2 VF requests */
> +#define IXGBE_VF_SET_MC_PROMISC 0x0a /* VF requests PF to set
> MC promiscuous */
> +
> /* GET_QUEUES return data indices within the mailbox */
> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues
> supported */
> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues
> supported */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index c76ba90..c19b7b8 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -108,9 +108,12 @@ static int __ixgbe_enable_sriov(struct ixgbe_adapter
> *adapter)
> adapter->flags2 &= ~(IXGBE_FLAG2_RSC_CAPABLE |
> IXGBE_FLAG2_RSC_ENABLED);
>
> - /* enable spoof checking for all VFs */
> - for (i = 0; i < adapter->num_vfs; i++)
> + for (i = 0; i < adapter->num_vfs; i++) {
> + /* Enable spoof checking for all VFs */
> adapter->vfinfo[i].spoofchk_enabled = true;
> + /* Turn multicast promiscuous mode off for all VFs */
> + adapter->vfinfo[i].mc_promisc = false;
> + }
> return 0;
> }
>
> @@ -311,6 +314,40 @@ int ixgbe_pci_sriov_configure(struct pci_dev *dev,
> int num_vfs)
> return ixgbe_pci_sriov_enable(dev, num_vfs); }
>
> +static int ixgbe_enable_vf_mc_promisc(struct ixgbe_adapter *adapter,
> +u32 vf) {
> + struct ixgbe_hw *hw;
> + u32 vmolr;
> +
> + hw = &adapter->hw;
> + vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> +
> + e_info(drv, "VF %u: enabling multicast promiscuous\n", vf);
> +
> + vmolr |= IXGBE_VMOLR_MPE;
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> +
> + return 0;
> +}
> +
> +static int ixgbe_disable_vf_mc_promisc(struct ixgbe_adapter *adapter,
> +u32 vf) {
> + struct ixgbe_hw *hw;
> + u32 vmolr;
> +
> + hw = &adapter->hw;
> + vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
> +
> + e_info(drv, "VF %u: disabling multicast promiscuous\n", vf);
> +
> + vmolr &= ~IXGBE_VMOLR_MPE;
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
> +
> + return 0;
> +}
> +
> static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
> u32 *msgbuf, u32 vf)
> {
> @@ -325,6 +362,12 @@ static int ixgbe_set_vf_multicasts(struct
> ixgbe_adapter *adapter,
> u32 mta_reg;
> u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
>
> + /* Disable multicast promiscuous first */
> + if (adapter->vfinfo[vf].mc_promisc) {
> + ixgbe_disable_vf_mc_promisc(adapter, vf);
> + adapter->vfinfo[vf].mc_promisc = false;
> + }
> +
> /* only so many hash values supported */
> entries = min(entries, IXGBE_MAX_VF_MC_ENTRIES);
>
> @@ -427,6 +470,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter
> *adapter, u32 *msgbuf, u32 vf) #endif /* CONFIG_FCOE */
> switch (adapter->vfinfo[vf].vf_api) {
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> /*
> * Version 1.1 supports jumbo frames on VFs if PF has
> * jumbo frames enabled which means legacy VFs are
> @@ -710,6 +754,12 @@ static int ixgbe_vf_reset_msg(struct ixgbe_adapter
> *adapter, u32 vf)
> IXGBE_WRITE_REG(hw, IXGBE_PVFTDWBALn(q_per_pool,
> vf, i), 0);
> }
>
> + /* Disable multicast promiscuous at reset */
> + if (adapter->vfinfo[vf].mc_promisc) {
> + ixgbe_disable_vf_mc_promisc(adapter, vf);
> + adapter->vfinfo[vf].mc_promisc = false;
> + }
> +
> /* reply to reset with ack and vf mac address */
> msgbuf[0] = IXGBE_VF_RESET;
> if (!is_zero_ether_addr(vf_mac)) {
> @@ -894,6 +944,12 @@ static int ixgbe_negotiate_vf_api(struct
> ixgbe_adapter *adapter,
> switch (api) {
> case ixgbe_mbox_api_10:
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> + e_info(drv, "VF %d requested api_version %s\n", vf,
> + (api == ixgbe_mbox_api_12) ? "ixgbe_mbox_api_12"
> :
> + (api == ixgbe_mbox_api_11) ? "ixgbe_mbox_api_11"
> :
> + (api == ixgbe_mbox_api_10) ? "ixgbe_mbox_api_10"
> :
> + "unknown");
> adapter->vfinfo[vf].vf_api = api;
> return 0;
> default:
> @@ -917,6 +973,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter
> *adapter,
> switch (adapter->vfinfo[vf].vf_api) {
> case ixgbe_mbox_api_20:
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> break;
> default:
> return -1;
> @@ -944,6 +1001,31 @@ static int ixgbe_get_vf_queues(struct
> ixgbe_adapter *adapter,
> return 0;
> }
>
> +static int ixgbe_set_vf_mc_promisc(struct ixgbe_adapter *adapter,
> + u32 *msgbuf, u32 vf)
> +{
> + bool enable = !!msgbuf[1]; /* msgbuf contains the flag to enable
> */
> +
> + switch (adapter->vfinfo[vf].vf_api) {
> + case ixgbe_mbox_api_12:
> + break;
> + default:
> + return -1;
> + }
> +
> + /* nothing to do */
> + if (adapter->vfinfo[vf].mc_promisc == enable)
> + return 0;
> +
> + adapter->vfinfo[vf].mc_promisc = enable;
> +
> + if (enable)
> + return ixgbe_enable_vf_mc_promisc(adapter, vf);
> + else
> + return ixgbe_disable_vf_mc_promisc(adapter, vf); }
> +
> +
> static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf) {
> u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
> @@ -1000,6 +1082,9 @@ static int ixgbe_rcv_msg_from_vf(struct
> ixgbe_adapter *adapter, u32 vf)
> case IXGBE_VF_GET_QUEUES:
> retval = ixgbe_get_vf_queues(adapter, msgbuf, vf);
> break;
> + case IXGBE_VF_SET_MC_PROMISC:
> + retval = ixgbe_set_vf_mc_promisc(adapter, msgbuf, vf);
> + break;
> default:
> e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
> retval = IXGBE_ERR_MBX;
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 62a0d8e..0403e1d 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct
> ixgbevf_adapter *adapter) static void ixgbevf_negotiate_api(struct
> ixgbevf_adapter *adapter) {
> struct ixgbe_hw *hw = &adapter->hw;
> - int api[] = { ixgbe_mbox_api_11,
> + int api[] = { ixgbe_mbox_api_12,
> + ixgbe_mbox_api_11,
> ixgbe_mbox_api_10,
> ixgbe_mbox_api_unknown };
> int err = 0, idx = 0;
> @@ -1894,6 +1895,12 @@ static void ixgbevf_negotiate_api(struct
> ixgbevf_adapter *adapter)
> idx++;
> }
>
> + dev_info(&adapter->pdev->dev, "mbox api_version = %s\n",
> + (hw->api_version == ixgbe_mbox_api_12) ?
> "ixgbe_mbox_api_12" :
> + (hw->api_version == ixgbe_mbox_api_11) ?
> "ixgbe_mbox_api_11" :
> + (hw->api_version == ixgbe_mbox_api_10) ?
> "ixgbe_mbox_api_10" :
> + "unknown");
> +
> spin_unlock_bh(&adapter->mbx_lock);
> }
>
> @@ -2072,6 +2079,9 @@ void ixgbevf_down(struct ixgbevf_adapter
> *adapter)
>
> netif_carrier_off(netdev);
>
> + /* drop multicast promiscuous mode flag */
> + adapter->hw.mac.mc_promisc = false;
> +
> if (!pci_channel_offline(adapter->pdev))
> ixgbevf_reset(adapter);
>
> @@ -3525,6 +3535,7 @@ static int ixgbevf_change_mtu(struct net_device
> *netdev, int new_mtu)
>
> switch (adapter->hw.api_version) {
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
> break;
> default:
> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h
> b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> index 0bc3005..62ef0d8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> @@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
> /* This value should always be last */
> ixgbe_mbox_api_unknown, /* indicates that API version is not
> known */
> };
> @@ -104,6 +105,9 @@ enum ixgbe_pfvf_api_rev {
> /* mailbox API, version 1.1 VF requests */
> #define IXGBE_VF_GET_QUEUE 0x09 /* get queue configuration */
>
> +/* mailbox API, version 1.2 VF requests */
> +#define IXGBE_VF_SET_MC_PROMISC 0x0a /* VF requests PF to set
> MC promiscuous */
> +
> /* GET_QUEUES return data indices within the mailbox */
> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues
> supported */
> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues
> supported */
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c
> b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index cdb53be..dfc87b0 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
> @@ -120,6 +120,9 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
> memcpy(hw->mac.perm_addr, addr, ETH_ALEN);
> hw->mac.mc_filter_type = msgbuf[IXGBE_VF_MC_TYPE_WORD];
>
> + /* after reset, MC promiscuous mode is disabled */
> + hw->mac.mc_promisc = false;
> +
> return 0;
> }
>
> @@ -327,8 +330,29 @@ static s32 ixgbevf_update_mc_addr_list_vf(struct
> ixgbe_hw *hw,
> */
>
> cnt = netdev_mc_count(netdev);
> - if (cnt > 30)
> + if (cnt > 30) {
> + /*
> + * If the API has the capability to handle MC promiscuous
> + * mode, turn it on.
> + */
> + if (hw->api_version == ixgbe_mbox_api_12) {
> + if (!hw->mac.mc_promisc) {
> + struct ixgbevf_adapter *adapter = hw->back;
> +
> + dev_info(&adapter->pdev->dev, "Request
> MC PROMISC\n");
> +
> + /* enabling multicast promiscuous */
> + msgbuf[0] = IXGBE_VF_SET_MC_PROMISC;
> + msgbuf[1] = 1;
> + ixgbevf_write_msg_read_ack(hw, msgbuf,
> 2);
> +
> + hw->mac.mc_promisc = true;
> + }
> +
> + return 0;
> + }
> cnt = 30;
> + }
> msgbuf[0] = IXGBE_VF_SET_MULTICAST;
> msgbuf[0] |= cnt << IXGBE_VT_MSGINFO_SHIFT;
>
> @@ -344,6 +368,8 @@ static s32 ixgbevf_update_mc_addr_list_vf(struct
> ixgbe_hw *hw,
>
> ixgbevf_write_msg_read_ack(hw, msgbuf,
> IXGBE_VFMAILBOX_SIZE);
>
> + hw->mac.mc_promisc = false;
> +
> return 0;
> }
>
> @@ -545,6 +571,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw,
> unsigned int *num_tcs,
> /* do nothing if API doesn't support ixgbevf_get_queues */
> switch (hw->api_version) {
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> break;
> default:
> return 0;
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h
> b/drivers/net/ethernet/intel/ixgbevf/vf.h
> index 5b17242..97790db 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
> @@ -87,6 +87,7 @@ struct ixgbe_mac_info {
> enum ixgbe_mac_type type;
>
> s32 mc_filter_type;
> + bool mc_promisc;
>
> bool get_link_status;
> u32 max_tx_queues;
> --
> 1.9.0
Thanks Hiroshi,
From a quick glance this looks good. Jeff will pull this into his queue so we can test it out.
-Don Skidmore <donald.c.skidmore@intel.com>
^ permalink raw reply
* Re: [PATCH] net/tulip: don't warn about unknown ARM architecture
From: Grant Grundler @ 2015-01-30 22:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: open list:TULIP NETWORK DRI..., David Miller, linux-arm-kernel,
open list, Grant Grundler
In-Reply-To: <1780413.LLpv9s01CB@wuerfel>
On Fri, Jan 30, 2015 at 1:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> ARM has 32-byte cache lines, which according to the comment in
> the init registers function seems to work best with the default
> value of 0x4800 that is also used on sparc and parisc.
>
> This adds ARM to the same list, to use that default but no
> longer warn about it.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Grant Grundler <grundler@parisc-linux.org>
thanks!
grant
> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
> index 6aa887e0e1cb..9beb3d34d4ba 100644
> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
> @@ -904,7 +904,7 @@ static void init_registers(struct net_device *dev)
> }
> #elif defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
> i |= 0xE000;
> -#elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC)
> +#elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
> i |= 0x4800;
> #else
> #warning Processor architecture undefined
>
^ permalink raw reply
* [PATCH] net/tulip: don't warn about unknown ARM architecture
From: Arnd Bergmann @ 2015-01-30 21:58 UTC (permalink / raw)
To: netdev; +Cc: David Miller, linux-arm-kernel, linux-kernel, Grant Grundler
ARM has 32-byte cache lines, which according to the comment in
the init registers function seems to work best with the default
value of 0x4800 that is also used on sparc and parisc.
This adds ARM to the same list, to use that default but no
longer warn about it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
index 6aa887e0e1cb..9beb3d34d4ba 100644
--- a/drivers/net/ethernet/dec/tulip/winbond-840.c
+++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
@@ -904,7 +904,7 @@ static void init_registers(struct net_device *dev)
}
#elif defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
i |= 0xE000;
-#elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC)
+#elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
i |= 0x4800;
#else
#warning Processor architecture undefined
^ permalink raw reply related
* [PATCH] net: hip04: add missing MODULE_LICENSE
From: Arnd Bergmann @ 2015-01-30 21:57 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Zhangfei Gao, Ding Tianhong
The hip04 ethernet driver causes a new compile-time warning
when built as a loadable module:
WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/hisilicon/hip04_eth.o
see include/linux/module.h for more information
This adds the license as "GPL", which matches the header of the file.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index c02b81bcfffb..b72d238695d7 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -968,3 +968,4 @@ static struct platform_driver hip04_mac_driver = {
module_platform_driver(hip04_mac_driver);
MODULE_DESCRIPTION("HISILICON P04 Ethernet driver");
+MODULE_LICENSE("GPL");
^ permalink raw reply related
* Re: [PATCH] net: restore lro after device leave forwarding state
From: Alexander Duyck @ 2015-01-30 20:48 UTC (permalink / raw)
To: Fan Du, bhutchings; +Cc: davem, netdev, fengyuleidian0615
In-Reply-To: <1422621209-23222-1-git-send-email-fan.du@intel.com>
On 01/30/2015 04:33 AM, Fan Du wrote:
> Either detaching a device from bridge or switching a device
> out of FORWARDING state, the original lro feature should
> possibly be enabled for good reason, e.g. hw feature like
> receive side coalescing could come into play.
>
> BEFORE:
> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
> large-receive-offload: off
>
> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
> large-receive-offload: off
>
> AFTER:
> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
> large-receive-offload: off
>
> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
> large-receive-offload: on
>
> Signed-off-by: Fan Du <fan.du@intel.com>
> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
First off this isn't a "fix". This is going to likely break more than
it fixes. The main reason why LRO is disabled is because it can cause
more harm then it helps. Since GRO is available we should err on the
side of caution since enabling LRO/RSC can have undesirable side effects
in a number of cases.
As far as the rest of the patch I have serious misgivings as this is
going to be switching on LRO in multiple cases so if the user disables
it they will find it was re-enabled when they likely weren't expecting
it. LRO/RSC has a history of causing issues in a number of different
cases. I'd say if it is off leave it off. If the user really wants to
enable it they can do so via the ethtool interface that is already
provided in the kernel after they have removed the interface from the
bridge, or disabled IP routing.
Leaving it disabled would be consistent with how it is handled in
ixgbe_fix_features when Rx checksum offload is disabled. We just
disable the feature and expect the user to re-enable it when they
re-enable Rx checksum offload. The same logic should apply here. The
user put the hardware in this state, it is the responsibility of the
user to sort out the side effects of disabled features after they revert
to their original state.
> ---
> include/linux/netdevice.h | 1 +
> net/bridge/br_if.c | 1 +
> net/core/dev.c | 24 ++++++++++++++++++++++++
> net/ipv4/devinet.c | 4 ++++
> net/ipv6/addrconf.c | 2 ++
> 5 files changed, 32 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 642d426..904b1a4 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2153,6 +2153,7 @@ int dev_alloc_name(struct net_device *dev, const char *name);
> int dev_open(struct net_device *dev);
> int dev_close(struct net_device *dev);
> void dev_disable_lro(struct net_device *dev);
> +void dev_enable_lro(struct net_device *dev);
> int dev_loopback_xmit(struct sk_buff *newskb);
> int dev_queue_xmit(struct sk_buff *skb);
> int dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 81e49fb..4236f3a 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -565,6 +565,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
> call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
>
> netdev_update_features(br->dev);
> + dev_enable_lro(dev);
>
> return 0;
> }
Removing an interface from a bridge should not be grounds for turning on
LRO/RSC.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1e325ad..938d7f6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1451,6 +1451,30 @@ void dev_disable_lro(struct net_device *dev)
> }
> EXPORT_SYMBOL(dev_disable_lro);
>
> +/**
> + * dev_enable_lro - enable Large Receive Offload on a device
> + * @dev: device
> + *
> + * Enable Large Receive Offload (LRO) on a net device. Must be
> + * called under RTNL. This is needed if device is not attached
> + * to a bridge, or user change the forwarding state.
> + */
> +void dev_enable_lro(struct net_device *dev)
> +{
> + struct net_device *lower_dev;
> + struct list_head *iter;
> +
> + dev->wanted_features |= NETIF_F_LRO;
> + netdev_update_features(dev);
> +
> + if (unlikely(!(dev->features & NETIF_F_LRO)))
> + netdev_WARN(dev, "failed to enable LRO!\n");
> +
> + netdev_for_each_lower_dev(dev, lower_dev, iter)
> + dev_enable_lro(lower_dev);
> +}
> +EXPORT_SYMBOL(dev_enable_lro);
> +
> static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val,
> struct net_device *dev)
> {
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 214882e..3307196 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1956,6 +1956,8 @@ static void inet_forward_change(struct net *net)
> struct in_device *in_dev;
> if (on)
> dev_disable_lro(dev);
> + else
> + dev_enable_lro(dev);
> rcu_read_lock();
> in_dev = __in_dev_get_rcu(dev);
> if (in_dev) {
Disabling it due to a feature conflict makes sense. Enabling it after
disabling forwarding not so much. If the user disabled it prior to
enabling forwarding it should stay disabled, not be enabled.
> @@ -2047,6 +2049,8 @@ static int devinet_sysctl_forward(struct ctl_table *ctl, int write,
> container_of(cnf, struct in_device, cnf);
> if (*valp)
> dev_disable_lro(idev->dev);
> + else
> + dev_enable_lro(idev->dev);
> inet_netconf_notify_devconf(net,
> NETCONFA_FORWARDING,
> idev->dev->ifindex,
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index f7c8bbe..4c3b54c 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -669,6 +669,8 @@ static void dev_forward_change(struct inet6_dev *idev)
> dev = idev->dev;
> if (idev->cnf.forwarding)
> dev_disable_lro(dev);
> + else
> + dev_enable_lro(dev);
> if (dev->flags & IFF_MULTICAST) {
> if (idev->cnf.forwarding) {
> ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
Same applies here.
If anything this patch seems like more of a feature request then a fix.
It would likely create a desirable effect for some, but have some
undesirable side-effects for other users.
- Alex
^ permalink raw reply
* Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]
From: Arvid Brodin @ 2015-01-30 20:00 UTC (permalink / raw)
To: Nicolas Dichtel, netdev; +Cc: davem, dmitry.tarnyagin, alex.aring, linux-wpan
In-Reply-To: <1422307694-10079-1-git-send-email-nicolas.dichtel@6wind.com>
On 2015-01-26 22:28, Nicolas Dichtel wrote:
*snip*
> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
> the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
> supports x-netns? If not, the newlink handler should use the dest_net instead
> of src_net, I can provide the patch.
*snip*
As the author of the HSR driver, I'd like to answer this question, but unfortunately
I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
helpful.
Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
but I'd still like to understand, if you have the time to explain.)
(Pretty much all I know about network namespaces is from this article:
http://lwn.net/Articles/580893/.)
--
Arvid Brodin | Consultant (Linux)
ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden
arvid.brodin@alten.se | www.alten.se/en/
^ permalink raw reply
* [PATCH] Documentation: Update netlink_mmap.txt
From: Richard Weinberger @ 2015-01-30 19:50 UTC (permalink / raw)
To: davem; +Cc: corbet, shemming, linux-doc, netdev, linux-kernel,
Richard Weinberger
Update netlink_mmap.txt wrt. commit 4682a0358639b29cf
("netlink: Always copy on mmap TX.").
Signed-off-by: Richard Weinberger <richard@nod.at>
---
Documentation/networking/netlink_mmap.txt | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/Documentation/networking/netlink_mmap.txt b/Documentation/networking/netlink_mmap.txt
index c6af4ba..30a95fd 100644
--- a/Documentation/networking/netlink_mmap.txt
+++ b/Documentation/networking/netlink_mmap.txt
@@ -199,16 +199,9 @@ frame header.
TX limitations
--------------
-Kernel processing usually involves validation of the message received by
-user-space, then processing its contents. The kernel must assure that
-userspace is not able to modify the message contents after they have been
-validated. In order to do so, the message is copied from the ring frame
-to an allocated buffer if either of these conditions is false:
-
-- only a single mapping of the ring exists
-- the file descriptor is not shared between processes
-
-This means that for threaded programs, the kernel will fall back to copying.
+As of Jan 2015 the message is always copied from the ring frame to an
+allocated buffer due to unresolved security concerns.
+See commit 4682a0358639b29cf ("netlink: Always copy on mmap TX.").
Example
-------
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next v4 0/7] switchdev offload flags
From: roopa @ 2015-01-30 19:49 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, vyasevic,
ronen.arad, netdev, davem, shm, gospo
In-Reply-To: <20150130165940.GA1872@nanopsycho.orion>
On 1/30/15, 8:59 AM, Jiri Pirko wrote:
> Fri, Jan 30, 2015 at 07:40:10AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch series introduces new offload flags for switchdev.
>> Kernel network subsystems can use this flag to accelerate
>> network functions by offloading to hw.
>>
>> I expect that there will be need for subsystem specific feature
>> flag in the future.
>>
>> This patch series currently only addresses bridge driver link
>> attribute offloads to hardware.
>>
>> Looking at the current state of bridge l2 offload in the kernel,
>> - flag 'self' is the way to directly manage the bridge device in hw via
>> the ndo_bridge_setlink/ndo_bridge_getlink calls
>>
>> - flag 'master' is always used to manage the in kernel bridge devices
>> via the same ndo_bridge_setlink/ndo_bridge_getlink calls
>>
>> Today these are used separately. The nic offloads use hwmode "vepa/veb" to go
>> directly to hw with the "self" flag.
>>
>> At this point i am trying not to introduce any new user facing flags/attributes.
>> In the model where we want the kernel bridging to be accelerated with
>> hardware, we very much want the bridge driver to be involved.
>>
>> In this proposal,
>> - The offload flag/bit helps switch asic drivers to indicate that they
>> accelerate the kernel networking objects/functions
>> - The user does not have to specify a new flag to do so. A bridge created with
>> switch asic ports will be accelerated if the switch driver supports it.
>> - The user can continue to directly manage l2 in nics (ixgbe) using the
>> existing hwmode/self flags
>> - It also does not stop users from using the 'self' flag to talk to the
>> switch asic driver directly
>> - Involving the bridge driver makes sure the add/del notifications to user
>> space go out after both kernel and hardware are programmed
>>
>> (To selectively offload bridge port attributes,
>> example learning in hw only etc, we can introduce offload bits for
>> per bridge port flag attribute as in my previous patch
>> https://patchwork.ozlabs.org/patch/413211/. I have not included that in this
>> series)
>>
>> v2
>> - try a different name for the offload flag/bit
>> - tries to solve the stacked netdev case by traversing the lowerdev
>> list to reach the switch port
>>
>> v3 -
>> - Tested with bond as bridge port for the stacked device case.
>> Includes a bond_fix_features change to not ignore the
>> NETIF_F_HW_NETFUNC_OFFLOAD flag
>> - Some checkpatch fixes
>>
>> v4 -
>> - rename flag to NETIF_F_HW_SWITCH_OFFLOAD
>> - add ndo_bridge_setlink/dellink handlers in bond and team drivers as
>> suggested by jiri.
>> - introduce default ndo_dflt_netdev_switch_port_bridge_setlink/dellink
>> handlers that masters can use to call offload api on lowerdevs.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Having a flu and being completely unusable, I gave this patchset a quick
> peek and looks allright. I will review once I recover (Sun/Mon).
> Not sure if Dave wants to hold the patchset until then.
>
>
ok sounds good, thanks, hope you feel better soon.
^ permalink raw reply
* [PATCH -next] net: dctcp: loosen requirement to assert ECT(0) during 3WHS
From: Florian Westphal @ 2015-01-30 19:45 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Daniel Borkmann, Glenn Judd
One deployment requirement of DCTCP is to be able to run
in a DC setting along with TCP traffic. As Glenn Judd's
NSDI'15 paper "Attaining the Promise and Avoiding the Pitfalls
of TCP in the Datacenter" [1] (tba) explains, one way to
solve this on switch side is to split DCTCP and TCP traffic
in two queues per switch port based on the DSCP: one queue
soley intended for DCTCP traffic and one for non-DCTCP traffic.
For the DCTCP queue, there's the marking threshold K as
explained in commit e3118e8359bb ("net: tcp: add DCTCP congestion
control algorithm") for RED marking ECT(0) packets with CE.
For the non-DCTCP queue, there's f.e. a classic tail drop queue.
As already explained in e3118e8359bb, running DCTCP at scale
when not marking SYN/SYN-ACK packets with ECT(0) has severe
consequences as for non-ECT(0) packets, traversing the RED
marking DCTCP queue will result in a severe reduction of
connection probability.
This is due to the DCTCP queue being dominated by ECT(0) traffic
and switches handle non-ECT traffic in the RED marking queue
after passing K as drops, where K is usually a low watermark
in order to leave enough tailroom for bursts. Splitting DCTCP
traffic among several queues (ECN and non-ECN queue) is being
considered a terrible idea in the network community as it
splits single flows across multiple network paths.
Therefore, commit e3118e8359bb implements this on Linux as
ECT(0) marked traffic, as we argue that marking all packets
of a DCTCP flow is the only viable solution and also doesn't
speak against the draft.
However, recently, a DCTCP implementation for FreeBSD hit also
their mainline kernel [2]. In order to let them play well
together with Linux' DCTCP, we would need to loosen the
requirement that ECT(0) has to be asserted during the 3WHS as
not implemented in FreeBSD. This simplifies the ECN test and
lets DCTCP work together with FreeBSD.
Joint work with Daniel Borkmann.
[1] https://www.usenix.org/conference/nsdi15/technical-sessions/presentation/judd
[2] https://github.com/freebsd/freebsd/commit/8ad879445281027858a7fa706d13e458095b595f
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Glenn Judd <glenn.judd@morganstanley.com>
---
net/ipv4/tcp_input.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 71fb37c..9ec9115 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5870,10 +5870,9 @@ static inline void pr_drop_req(struct request_sock *req, __u16 port, int family)
* TCP ECN negotiation.
*
* Exception: tcp_ca wants ECN. This is required for DCTCP
- * congestion control; it requires setting ECT on all packets,
- * including SYN. We inverse the test in this case: If our
- * local socket wants ECN, but peer only set ece/cwr (but not
- * ECT in IP header) its probably a non-DCTCP aware sender.
+ * congestion control: Linux DCTCP asserts ECT on all packets,
+ * including SYN, which is most optimal solution; however,
+ * others, such as FreeBSD do not.
*/
static void tcp_ecn_create_request(struct request_sock *req,
const struct sk_buff *skb,
@@ -5883,18 +5882,15 @@ static void tcp_ecn_create_request(struct request_sock *req,
const struct tcphdr *th = tcp_hdr(skb);
const struct net *net = sock_net(listen_sk);
bool th_ecn = th->ece && th->cwr;
- bool ect, need_ecn, ecn_ok;
+ bool ect, ecn_ok;
if (!th_ecn)
return;
ect = !INET_ECN_is_not_ect(TCP_SKB_CB(skb)->ip_dsfield);
- need_ecn = tcp_ca_needs_ecn(listen_sk);
ecn_ok = net->ipv4.sysctl_tcp_ecn || dst_feature(dst, RTAX_FEATURE_ECN);
- if (!ect && !need_ecn && ecn_ok)
- inet_rsk(req)->ecn_ok = 1;
- else if (ect && need_ecn)
+ if ((!ect && ecn_ok) || tcp_ca_needs_ecn(listen_sk))
inet_rsk(req)->ecn_ok = 1;
}
--
2.0.5
^ permalink raw reply related
* [PATCH v2 3/3] Revert "drivers/net: Disable UFO through virtio"
From: Vladislav Yasevich @ 2015-01-30 19:27 UTC (permalink / raw)
To: netdev; +Cc: virtualization, mst, ben, eric.dumazet, hannes,
Vladislav Yasevich
In-Reply-To: <1422646047-13168-1-git-send-email-vyasevic@redhat.com>
This reverts commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4.
Now that GSO functionality can correctly track if the fragment
id has been selected and select a fragment id if necessary,
we can re-enable UFO on tap/macvap and virtio devices.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/macvtap.c | 13 ++++++++-----
drivers/net/tun.c | 19 ++++++++-----------
drivers/net/virtio_net.c | 24 ++++++++++--------------
3 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 0b86e46..919f4fc 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -80,7 +80,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 | \
- NETIF_F_TSO6)
+ NETIF_F_TSO6 | NETIF_F_UFO)
#define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
#define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
@@ -585,8 +585,6 @@ static int macvtap_skb_from_vnet_hdr(struct macvtap_queue *q,
gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- pr_warn_once("macvtap: %s: using disabled UFO feature; please fix this program\n",
- current->comm);
gso_type = SKB_GSO_UDP;
break;
default:
@@ -633,6 +631,8 @@ static void macvtap_skb_to_vnet_hdr(struct macvtap_queue *q,
vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (sinfo->gso_type & SKB_GSO_UDP)
+ vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP;
else
BUG();
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
@@ -962,6 +962,9 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
if (arg & TUN_F_TSO6)
feature_mask |= NETIF_F_TSO6;
}
+
+ if (arg & TUN_F_UFO)
+ feature_mask |= NETIF_F_UFO;
}
/* tun/tap driver inverts the usage for TSO offloads, where
@@ -972,7 +975,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
* When user space turns off TSO, we turn off GSO/LRO so that
* user-space will not receive TSO frames.
*/
- if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
+ if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
features |= RX_OFFLOADS;
else
features &= ~RX_OFFLOADS;
@@ -1087,7 +1090,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
case TUNSETOFFLOAD:
/* let the user check for future flags */
if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
- TUN_F_TSO_ECN))
+ TUN_F_TSO_ECN | TUN_F_UFO))
return -EINVAL;
rtnl_lock();
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5ca42b7..10f9e40 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -186,7 +186,7 @@ struct tun_struct {
struct net_device *dev;
netdev_features_t set_features;
#define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
- NETIF_F_TSO6)
+ NETIF_F_TSO6|NETIF_F_UFO)
int vnet_hdr_sz;
int sndbuf;
@@ -1176,18 +1176,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- {
- static bool warned;
-
- if (!warned) {
- warned = true;
- netdev_warn(tun->dev,
- "%s: using disabled UFO feature; please fix this program\n",
- current->comm);
- }
skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
break;
- }
default:
tun->dev->stats.rx_frame_errors++;
kfree_skb(skb);
@@ -1294,6 +1284,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (sinfo->gso_type & SKB_GSO_UDP)
+ gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
else {
pr_err("unexpected GSO type: "
"0x%x, gso_size %d, hdr_len %d\n",
@@ -1742,6 +1734,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)
features |= NETIF_F_TSO6;
arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
}
+
+ if (arg & TUN_F_UFO) {
+ features |= NETIF_F_UFO;
+ arg &= ~TUN_F_UFO;
+ }
}
/* This gives the user a way to test for new features in future by
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5ca9771..059fdf1 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -490,17 +490,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- {
- static bool warned;
-
- if (!warned) {
- warned = true;
- netdev_warn(dev,
- "host using disabled UFO feature; please fix it\n");
- }
skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
break;
- }
case VIRTIO_NET_HDR_GSO_TCPV6:
skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
break;
@@ -888,6 +879,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
+ hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_UDP;
else
BUG();
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
@@ -1748,7 +1741,7 @@ static int virtnet_probe(struct virtio_device *vdev)
dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
- dev->hw_features |= NETIF_F_TSO
+ dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
| NETIF_F_TSO_ECN | NETIF_F_TSO6;
}
/* Individual feature bits: what can host handle? */
@@ -1758,9 +1751,11 @@ static int virtnet_probe(struct virtio_device *vdev)
dev->hw_features |= NETIF_F_TSO6;
if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
dev->hw_features |= NETIF_F_TSO_ECN;
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
+ dev->hw_features |= NETIF_F_UFO;
if (gso)
- dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
+ dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO);
/* (!csum && gso) case will be fixed by register_netdev() */
}
if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
@@ -1798,7 +1793,8 @@ static int virtnet_probe(struct virtio_device *vdev)
/* If we can receive ANY GSO packets, we must allocate large ones. */
if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
- virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
+ virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN) ||
+ virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UFO))
vi->big_packets = true;
if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
@@ -1994,9 +1990,9 @@ static struct virtio_device_id id_table[] = {
static unsigned int features[] = {
VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
- VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6,
+ VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
- VIRTIO_NET_F_GUEST_ECN,
+ VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
--
1.9.3
^ permalink raw reply related
* [PATCH v2 2/3] Revert "drivers/net, ipv6: Select IPv6 fragment idents for virtio UFO packets"
From: Vladislav Yasevich @ 2015-01-30 19:27 UTC (permalink / raw)
To: netdev; +Cc: virtualization, mst, ben, eric.dumazet, hannes,
Vladislav Yasevich
In-Reply-To: <1422646047-13168-1-git-send-email-vyasevic@redhat.com>
This reverts commit 5188cd44c55db3e92cd9e77a40b5baa7ed4340f7.
Now that GSO layer can track if fragment id has been selected
and can allocate one if necessary, we don't need to do this in
tap and macvtap. This reverts most of the code and only keeps
the new ipv6 fragment id generation function that is still needed.
Fixes: 3d0ad09412ff (drivers/net: Disable UFO through virtio)
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/macvtap.c | 3 ---
drivers/net/tun.c | 6 +-----
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7df2217..0b86e46 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -17,7 +17,6 @@
#include <linux/fs.h>
#include <linux/uio.h>
-#include <net/ipv6.h>
#include <net/net_namespace.h>
#include <net/rtnetlink.h>
#include <net/sock.h>
@@ -589,8 +588,6 @@ static int macvtap_skb_from_vnet_hdr(struct macvtap_queue *q,
pr_warn_once("macvtap: %s: using disabled UFO feature; please fix this program\n",
current->comm);
gso_type = SKB_GSO_UDP;
- if (skb->protocol == htons(ETH_P_IPV6))
- ipv6_proxy_select_ident(skb);
break;
default:
return -EINVAL;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 8c8dc16..5ca42b7 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -65,7 +65,6 @@
#include <linux/nsproxy.h>
#include <linux/virtio_net.h>
#include <linux/rcupdate.h>
-#include <net/ipv6.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
#include <net/rtnetlink.h>
@@ -1167,8 +1166,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
break;
}
- skb_reset_network_header(skb);
-
if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
pr_debug("GSO!\n");
switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
@@ -1189,8 +1186,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
current->comm);
}
skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
- if (skb->protocol == htons(ETH_P_IPV6))
- ipv6_proxy_select_ident(skb);
break;
}
default:
@@ -1221,6 +1216,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
}
+ skb_reset_network_header(skb);
skb_probe_transport_header(skb, 0);
rxhash = skb_get_hash(skb);
--
1.9.3
^ permalink raw reply related
* [PATCH v2 1/3] ipv6: Select fragment id during UFO segmentation if not set.
From: Vladislav Yasevich @ 2015-01-30 19:27 UTC (permalink / raw)
To: netdev; +Cc: virtualization, mst, ben, eric.dumazet, hannes,
Vladislav Yasevich
In-Reply-To: <1422646047-13168-1-git-send-email-vyasevic@redhat.com>
If the IPv6 fragment id has not been set and we perform
fragmentation due to UFO, select a new fragment id.
We now consider a fragment id of 0 as unset and if id selection
process returns 0 (after all the pertrubations), we set it to
0x80000000, thus giving us ample space not to create collisions
with the next packet we may have to fragment.
When doing UFO integrity checking, we also select the
fragment id if it has not be set yet. This is stored into
the skb_shinfo() thus allowing UFO to function correclty.
This patch also removes duplicate fragment id generation code
and moves ipv6_select_ident() into the header as it may be
used during GSO.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
include/net/ipv6.h | 3 +++
net/ipv6/ip6_output.c | 10 ++++------
net/ipv6/output_core.c | 28 ++++++++++++++++++++++------
net/ipv6/udp_offload.c | 10 +++++++++-
4 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 4292929..9bf85d3 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -671,6 +671,9 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
}
+u32 __ipv6_select_ident(u32 hashrnd, struct in6_addr *dst,
+ struct in6_addr *src);
+void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
void ipv6_proxy_select_ident(struct sk_buff *skb);
int ip6_dst_hoplimit(struct dst_entry *dst);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index ce69a12..5dc91fe 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -537,17 +537,15 @@ static void ip6_copy_metadata(struct sk_buff *to, struct sk_buff *from)
skb_copy_secmark(to, from);
}
-static void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
+void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
{
static u32 ip6_idents_hashrnd __read_mostly;
- u32 hash, id;
+ u32 id;
net_get_random_once(&ip6_idents_hashrnd, sizeof(ip6_idents_hashrnd));
- hash = __ipv6_addr_jhash(&rt->rt6i_dst.addr, ip6_idents_hashrnd);
- hash = __ipv6_addr_jhash(&rt->rt6i_src.addr, hash);
-
- id = ip_idents_reserve(hash, 1);
+ id = __ipv6_select_ident(ip6_idents_hashrnd, &rt->rt6i_dst.addr,
+ &rt->rt6i_src.addr);
fhdr->identification = htonl(id);
}
diff --git a/net/ipv6/output_core.c b/net/ipv6/output_core.c
index 97f41a3..ceb857d 100644
--- a/net/ipv6/output_core.c
+++ b/net/ipv6/output_core.c
@@ -9,6 +9,24 @@
#include <net/addrconf.h>
#include <net/secure_seq.h>
+u32 __ipv6_select_ident(u32 hashrnd, struct in6_addr *dst, struct in6_addr *src)
+{
+ u32 hash, id;
+
+ hash = __ipv6_addr_jhash(dst, hashrnd);
+ hash = __ipv6_addr_jhash(src, hash);
+
+ /* Treat id of 0 as unset and if we get 0 back from ip_idents_reserve,
+ * set the hight order instead thus minimizing possible future
+ * collisions.
+ */
+ id = ip_idents_reserve(hash, 1);
+ if (unlikely(!id))
+ id = 1 << 31;
+
+ return id;
+}
+
/* This function exists only for tap drivers that must support broken
* clients requesting UFO without specifying an IPv6 fragment ID.
*
@@ -22,7 +40,7 @@ void ipv6_proxy_select_ident(struct sk_buff *skb)
static u32 ip6_proxy_idents_hashrnd __read_mostly;
struct in6_addr buf[2];
struct in6_addr *addrs;
- u32 hash, id;
+ u32 id;
addrs = skb_header_pointer(skb,
skb_network_offset(skb) +
@@ -34,11 +52,9 @@ void ipv6_proxy_select_ident(struct sk_buff *skb)
net_get_random_once(&ip6_proxy_idents_hashrnd,
sizeof(ip6_proxy_idents_hashrnd));
- hash = __ipv6_addr_jhash(&addrs[1], ip6_proxy_idents_hashrnd);
- hash = __ipv6_addr_jhash(&addrs[0], hash);
-
- id = ip_idents_reserve(hash, 1);
- skb_shinfo(skb)->ip6_frag_id = htonl(id);
+ id = __ipv6_select_ident(ip6_proxy_idents_hashrnd,
+ &addrs[1], &addrs[0]);
+ skb_shinfo(skb)->ip6_frag_id = id;
}
EXPORT_SYMBOL_GPL(ipv6_proxy_select_ident);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index b6aa8ed..a562769 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -52,6 +52,10 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
+ /* Set the IPv6 fragment id if not set yet */
+ if (!skb_shinfo(skb)->ip6_frag_id)
+ ipv6_proxy_select_ident(skb);
+
segs = NULL;
goto out;
}
@@ -108,7 +112,11 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
fptr->nexthdr = nexthdr;
fptr->reserved = 0;
- fptr->identification = skb_shinfo(skb)->ip6_frag_id;
+ if (skb_shinfo(skb)->ip6_frag_id)
+ fptr->identification = skb_shinfo(skb)->ip6_frag_id;
+ else
+ ipv6_select_ident(fptr,
+ (struct rt6_info *)skb_dst(skb));
/* Fragment the skb. ipv6 header and the remaining fields of the
* fragment header are updated in ipv6_gso_segment()
--
1.9.3
^ permalink raw reply related
* [PATCH v2 0/3] Restore UFO support to virtio_net devices
From: Vladislav Yasevich @ 2015-01-30 19:27 UTC (permalink / raw)
To: netdev; +Cc: virtualization, mst, ben, eric.dumazet, hannes,
Vladislav Yasevich
commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4
Author: Ben Hutchings <ben@decadent.org.uk>
Date: Thu Oct 30 18:27:12 2014 +0000
drivers/net: Disable UFO through virtio
Turned off UFO support to virtio-net based devices due to issues
with IPv6 fragment id generation for UFO packets. The issue
was that IPv6 UFO/GSO implementation expects the fragment id
to be supplied in skb_shinfo(). However, for packets generated
by the VMs, the fragment id is not supplied which causes all
IPv6 fragments to have the id of 0.
The problem is that turning off UFO support on tap/macvtap
as well as virtio devices caused issues with migrations.
Migrations would fail when moving a vm from a kernel supporting
expecting UFO to work to the newer kernels that disabled UFO.
This series provides a partial solution to address the migration
issue. The series allows us to track whether skb_shinfo()->ip6_frag_id
has been set by treating value of 0 as unset.
This lets GSO code to generate fragment ids if they are necessary
(ex: packet was generated by VM or packet socket).
Since v1:
- Removed the skb bit and use value of 0 as tracker.
- Used Eric's suggestion to set fragment id as 0x80000000 if id
generation procedure yeilded a 0 result.
- Consolidated ipv6 id genration code.
Vladislav Yasevich (3):
ipv6: Select fragment id during UFO segmentation if not set.
Revert "drivers/net, ipv6: Select IPv6 fragment idents for virtio UFO
packets"
Revert "drivers/net: Disable UFO through virtio"
drivers/net/macvtap.c | 16 ++++++++--------
drivers/net/tun.c | 25 +++++++++----------------
drivers/net/virtio_net.c | 24 ++++++++++--------------
include/net/ipv6.h | 3 +++
net/ipv6/ip6_output.c | 10 ++++------
net/ipv6/output_core.c | 28 ++++++++++++++++++++++------
net/ipv6/udp_offload.c | 10 +++++++++-
7 files changed, 65 insertions(+), 51 deletions(-)
--
1.9.3
^ permalink raw reply
* Re: [PATCH net] netlink: fix wrong subscription bitmask to group mapping in
From: Pablo Neira Ayuso @ 2015-01-30 19:26 UTC (permalink / raw)
To: Ivan Delalande; +Cc: netdev, davem, andre
In-Reply-To: <20150129222701.GP17877@ycc.fr>
On Thu, Jan 29, 2015 at 11:27:01PM +0100, Ivan Delalande wrote:
> On Thu, Jan 29, 2015 at 10:51:53AM +0100, Pablo Neira Ayuso wrote:
> > The subscription bitmask passed via struct sockaddr_nl is converted to
> > the group number when calling the netlink_bind() and netlink_unbind()
> > callbacks.
> >
> > The conversion is however incorrect since bitmask (1 << 0) needs to be
> > mapped to group number 1. Note that you cannot specify the group number 0
> > (usually known as _NONE) from setsockopt() using NETLINK_ADD_MEMBERSHIP
> > since this is rejected through -EINVAL.
> >
> > This problem became noticeable since 97840cb ("netfilter: nfnetlink:
> > fix insufficient validation in nfnetlink_bind") when binding to bitmask
> > (1 << 0) in ctnetlink.
> >
> > Reported-by: Andre Tomt <andre@tomt.net>
> > Reported-by: Ivan Delalande <colona@arista.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Thanks a lot for this fix!
>
> > ---
> > v2: Rebased upon current net tree. Previous patch:
> >
> > http://patchwork.ozlabs.org/patch/426205/
> >
> > did not apply cleanly.
> >
> > net/netlink/af_netlink.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> > index 02fdde2..75532ef 100644
> > --- a/net/netlink/af_netlink.c
> > +++ b/net/netlink/af_netlink.c
> > @@ -1438,7 +1438,7 @@ static void netlink_undo_bind(int group, long unsigned int groups,
> >
> > for (undo = 0; undo < group; undo++)
> > if (test_bit(undo, &groups))
> > - nlk->netlink_unbind(sock_net(sk), undo);
> > + nlk->netlink_unbind(sock_net(sk), undo + 1);
> > }
> >
> > static int netlink_bind(struct socket *sock, struct sockaddr *addr,
> > @@ -1476,7 +1476,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
> > for (group = 0; group < nlk->ngroups; group++) {
> > if (!test_bit(group, &groups))
> > continue;
> > - err = nlk->netlink_bind(net, group);
> > + err = nlk->netlink_bind(net, group + 1);
> > if (!err)
> > continue;
> > netlink_undo_bind(group, groups, sk);
>
> I guess this should also be group + 1 there:
>
> netlink_undo_bind(group + 1, groups, sk);
We don't need that change you suggest.
Asumming nlk->netlink_bind(...) fails when 'group' is 0, then we have
nothing to undo.
See netlink_undo_bind():
for (undo = 0; undo < group; undo++)
if (test_bit(undo, &groups))
nlk->netlink_unbind(sock_net(sk), undo + 1);
'undo' and 'group' will be both 0, so nothing is undone as expected.
So my patch seems good to me after second look. Let me know if you
have more concerns, thanks.
^ 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