* Re: [Intel-wired-lan] [PATCH v2] PCI: lock each enable/disable num_vfs operation in sysfs
From: Greg @ 2017-01-06 23:35 UTC (permalink / raw)
To: Tantilov, Emil S
Cc: linux-pci@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Gavin Shan
In-Reply-To: <87618083B2453E4A8714035B62D6799250744DCD@FMSMSX105.amr.corp.intel.com>
On Fri, 2017-01-06 at 23:28 +0000, Tantilov, Emil S wrote:
> >-----Original Message-----
> >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> >Behalf Of Greg
> >Sent: Friday, January 06, 2017 3:18 PM
> >To: Tantilov, Emil S <emil.s.tantilov@intel.com>
> >Cc: linux-pci@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> >kernel@vger.kernel.org; netdev@vger.kernel.org
> >Subject: Re: [Intel-wired-lan] [PATCH v2] PCI: lock each enable/disable
> >num_vfs operation in sysfs
> >
> >On Fri, 2017-01-06 at 13:59 -0800, Emil Tantilov wrote:
> >> Enabling/disabling SRIOV via sysfs by echo-ing multiple values
> >> simultaneously:
> >>
> >> echo 63 > /sys/class/net/ethX/device/sriov_numvfs&
> >> echo 63 > /sys/class/net/ethX/device/sriov_numvfs
> >>
> >> sleep 5
> >>
> >> echo 0 > /sys/class/net/ethX/device/sriov_numvfs&
> >> echo 0 > /sys/class/net/ethX/device/sriov_numvfs
> >>
> >> Results in the following bug:
> >>
> >> kernel BUG at drivers/pci/iov.c:495!
> >> invalid opcode: 0000 [#1] SMP
> >> CPU: 1 PID: 8050 Comm: bash Tainted: G W 4.9.0-rc7-net-next #2092
> >> RIP: 0010:[<ffffffff813b1647>]
> >> [<ffffffff813b1647>] pci_iov_release+0x57/0x60
> >>
> >> Call Trace:
> >> [<ffffffff81391726>] pci_release_dev+0x26/0x70
> >> [<ffffffff8155be6e>] device_release+0x3e/0xb0
> >> [<ffffffff81365ee7>] kobject_cleanup+0x67/0x180
> >> [<ffffffff81365d9d>] kobject_put+0x2d/0x60
> >> [<ffffffff8155bc27>] put_device+0x17/0x20
> >> [<ffffffff8139c08a>] pci_dev_put+0x1a/0x20
> >> [<ffffffff8139cb6b>] pci_get_dev_by_id+0x5b/0x90
> >> [<ffffffff8139cca5>] pci_get_subsys+0x35/0x40
> >> [<ffffffff8139ccc8>] pci_get_device+0x18/0x20
> >> [<ffffffff8139ccfb>] pci_get_domain_bus_and_slot+0x2b/0x60
> >> [<ffffffff813b09e7>] pci_iov_remove_virtfn+0x57/0x180
> >> [<ffffffff813b0b95>] pci_disable_sriov+0x65/0x140
> >> [<ffffffffa00a1af7>] ixgbe_disable_sriov+0xc7/0x1d0 [ixgbe]
> >> [<ffffffffa00a1e9d>] ixgbe_pci_sriov_configure+0x3d/0x170 [ixgbe]
> >> [<ffffffff8139d28c>] sriov_numvfs_store+0xdc/0x130
> >> ...
> >> RIP [<ffffffff813b1647>] pci_iov_release+0x57/0x60
> >>
> >> Use the existing mutex lock to protect each enable/disable operation.
> >>
> >> -v2: move the existing lock from protecting the config of the IOV bus
> >> to protecting the writes to sriov_numvfs in sysfs without maintaining
> >> a "locked" version of pci_iov_add/remove_virtfn().
> >> As suggested by Gavin Shan <gwshan@linux.vnet.ibm.com>
> >>
> >> CC: Alexander Duyck <alexander.h.duyck@intel.com>
> >> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> >> ---
> >> drivers/pci/iov.c | 7 -------
> >> drivers/pci/pci-sysfs.c | 23 ++++++++++++++++-------
> >> drivers/pci/pci.h | 2 +-
> >> 3 files changed, 17 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> >> index 4722782..2479ae8 100644
> >> --- a/drivers/pci/iov.c
> >> +++ b/drivers/pci/iov.c
> >> @@ -124,7 +124,6 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id,
> >int reset)
> >> struct pci_sriov *iov = dev->sriov;
> >> struct pci_bus *bus;
> >>
> >> - mutex_lock(&iov->dev->sriov->lock);
> >> bus = virtfn_add_bus(dev->bus, pci_iov_virtfn_bus(dev, id));
> >> if (!bus)
> >> goto failed;
> >> @@ -162,7 +161,6 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id,
> >int reset)
> >> __pci_reset_function(virtfn);
> >>
> >> pci_device_add(virtfn, virtfn->bus);
> >> - mutex_unlock(&iov->dev->sriov->lock);
> >>
> >> pci_bus_add_device(virtfn);
> >> sprintf(buf, "virtfn%u", id);
> >> @@ -181,12 +179,10 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id,
> >int reset)
> >> sysfs_remove_link(&dev->dev.kobj, buf);
> >> failed1:
> >> pci_dev_put(dev);
> >> - mutex_lock(&iov->dev->sriov->lock);
> >> pci_stop_and_remove_bus_device(virtfn);
> >> failed0:
> >> virtfn_remove_bus(dev->bus, bus);
> >> failed:
> >> - mutex_unlock(&iov->dev->sriov->lock);
> >>
> >> return rc;
> >> }
> >> @@ -195,7 +191,6 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int
> >id, int reset)
> >> {
> >> char buf[VIRTFN_ID_LEN];
> >> struct pci_dev *virtfn;
> >> - struct pci_sriov *iov = dev->sriov;
> >>
> >> virtfn = pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> >> pci_iov_virtfn_bus(dev, id),
> >> @@ -218,10 +213,8 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int
> >id, int reset)
> >> if (virtfn->dev.kobj.sd)
> >> sysfs_remove_link(&virtfn->dev.kobj, "physfn");
> >>
> >> - mutex_lock(&iov->dev->sriov->lock);
> >> pci_stop_and_remove_bus_device(virtfn);
> >> virtfn_remove_bus(dev->bus, virtfn->bus);
> >> - mutex_unlock(&iov->dev->sriov->lock);
> >>
> >> /* balance pci_get_domain_bus_and_slot() */
> >> pci_dev_put(virtfn);
> >> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> >> index 0666287..25d010d 100644
> >> --- a/drivers/pci/pci-sysfs.c
> >> +++ b/drivers/pci/pci-sysfs.c
> >> @@ -472,6 +472,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
> >> const char *buf, size_t count)
> >> {
> >> struct pci_dev *pdev = to_pci_dev(dev);
> >> + struct pci_sriov *iov = pdev->sriov;
> >> int ret;
> >> u16 num_vfs;
> >>
> >> @@ -482,38 +483,46 @@ static ssize_t sriov_numvfs_store(struct device
> >*dev,
> >> if (num_vfs > pci_sriov_get_totalvfs(pdev))
> >> return -ERANGE;
> >>
> >> + mutex_lock(&iov->dev->sriov->lock);
> >> +
> >> if (num_vfs == pdev->sriov->num_VFs)
> >> - return count; /* no change */
> >> + goto exit;
> >>
> >> /* is PF driver loaded w/callback */
> >> if (!pdev->driver || !pdev->driver->sriov_configure) {
> >> dev_info(&pdev->dev, "Driver doesn't support SRIOV
> >configuration via sysfs\n");
> >> - return -ENOSYS;
> >> + ret = -ENOENT;
> >> + goto exit;
> >
> >Hi Emil,
> >
> >Generally the patch looks fine to me - but I am wondering why the change
> >from -ENOSYS TO -ENOENT?
>
> Because checkpatch was complaining that ENOSYS should only be used for
> "invalid syscall nr".
>
> If you look at the discussion of the previous version of the patch you can
> see the details there. Basically Gavin preferred ENOENT and I don't really
> care so much (I kind of ran into this by accident as this patch does not need
> to change the error code).
Ah, OK. Thanks for the explanation! Tell all my old buddies there at
Intel I said hi.
Regards,
- Greg
>
> Thanks,
> Emil
^ permalink raw reply
* RE: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-06 23:38 UTC (permalink / raw)
To: Joao Pinto, David S. Miller, Giuseppe CAVALLARO,
seraphin.bonnaffe@st.com, Jarod Wilson, Andy Shevchenko
Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
pavel@ucw.cz, lars.persson@axis.com, netdev, LKML
In-Reply-To: <a7c3212f-1772-8e07-b9a3-3ebbb4150fc7@synopsys.com>
> -----Original Message-----
> From: Joao Pinto [mailto:Joao.Pinto@synopsys.com]
> Sent: Saturday, January 07, 2017 12:58 AM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>; David S. Miller
> <davem@davemloft.net>; Joao Pinto <Joao.Pinto@synopsys.com>; Giuseppe
> CAVALLARO <peppe.cavallaro@st.com>; seraphin.bonnaffe@st.com; Jarod
> Wilson <jarod@redhat.com>; Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Alexandre TORGUE <alexandre.torgue@gmail.com>; Joachim Eastwood
> <manabian@gmail.com>; Niklas Cassel <niklas.cassel@axis.com>; Johan Hovold
> <johan@kernel.org>; pavel@ucw.cz; lars.persson@axis.com; netdev
> <netdev@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid
> range
>
>
> Hi Wilson,
>
> Às 12:49 AM de 1/7/2017, Kweh, Hock Leong escreveu:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > There is no checking valid value of maxmtu when getting it from device tree.
> > This resolution added the checking condition to ensure the assignment is
> > made within a valid range.
> >
> > Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> > ---
> > changelog v3:
> > * print the warning message only if maxmtu < min_mtu
> > * add maxmtu = JUMBO_LEN at stmmac_pci.c to follow stmmac_platform.c
> >
> > changelog v2:
> > * correction of "devicetree" to "device tree" reported by Andy
> > * print warning message while maxmtu is not in valid range
> >
> > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
> > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 92ac006..ce74ae6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
> > ndev->max_mtu = JUMBO_LEN;
> > else
> > ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD +
> NET_IP_ALIGN);
> > - if (priv->plat->maxmtu < ndev->max_mtu)
> > +
> > + if ((priv->plat->maxmtu < ndev->max_mtu) &&
> > + (priv->plat->maxmtu >= ndev->min_mtu))
> > ndev->max_mtu = priv->plat->maxmtu;
> > + else if (priv->plat->maxmtu < ndev->min_mtu)
> > + netdev_warn(priv->dev,
> > + "%s: warning: maxmtu having invalid value (%d)\n",
> > + __func__, priv->plat->maxmtu);
> >
> > if (flow_ctrl)
> > priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > index a283177..e539afe 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > @@ -204,6 +204,10 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
> >
> > pci_set_master(pdev);
> >
> > + /* Set the maxmtu to a default of JUMBO_LEN in case the
> > + * parameter is not defined for the device.
> > + */
> > + plat->maxmtu = JUMBO_LEN;
>
> I suggest to put this configuration in one of the default config functions.
>
> Tahnks.
>
My original thought is to have one line for all *_default_data() instead of having
multiple same line inside each *_default_data(). If this is not a big concern of
future expand, I can do that. Thanks.
> > if (info) {
> > info->pdev = pdev;
> > if (info->setup) {
> >
^ permalink raw reply
* Re: [PATCHv1 7/7] IPVTAP: IP-VLAN based tap driver
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-01-06 23:46 UTC (permalink / raw)
To: Sainath Grandhi; +Cc: linux-netdev, David Miller, mahesh, linux-kernel
In-Reply-To: <1483742009-19184-8-git-send-email-sainath.grandhi@intel.com>
few superficial comments inline.
On Fri, Jan 6, 2017 at 2:33 PM, Sainath Grandhi
<sainath.grandhi@intel.com> wrote:
> This patch adds a tap character device driver that is based on the
> IP-VLAN network interface, called ipvtap. An ipvtap device can be created
> in the same way as an ipvlan device, using 'type ipvtap', and then accessed
> using the tap user space interface.
>
> Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.com>
> Tested-by: Sainath Grandhi <sainath.grandhi@intel.com>
> ---
> drivers/net/Kconfig | 12 ++
> drivers/net/Makefile | 1 +
> drivers/net/ipvlan/Makefile | 1 +
> drivers/net/ipvlan/ipvlan.h | 7 ++
> drivers/net/ipvlan/ipvlan_core.c | 5 +-
> drivers/net/ipvlan/ipvlan_main.c | 37 +++---
> drivers/net/ipvlan/ipvtap.c | 238 +++++++++++++++++++++++++++++++++++++++
> 7 files changed, 282 insertions(+), 19 deletions(-)
> create mode 100644 drivers/net/ipvlan/ipvtap.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 280380d..ddfb30a 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -165,6 +165,18 @@ config IPVLAN
> To compile this driver as a module, choose M here: the module
> will be called ipvlan.
>
> +config IPVTAP
> + tristate "IP-VLAN based tap driver"
> + depends on IPVLAN
> + depends on INET
> + help
> + This adds a specialized tap character device driver that is based
> + on the IP-VLAN network interface, called ipvtap. An ipvtap device
> + can be added in the same way as a ipvlan device, using 'type
> + ipvtap', and then be accessed through the tap user space interface.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called macvtap.
>
> config VXLAN
> tristate "Virtual eXtensible Local Area Network (VXLAN)"
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 7dd86ca..98ed4d9 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -7,6 +7,7 @@
> #
> obj-$(CONFIG_BONDING) += bonding/
> obj-$(CONFIG_IPVLAN) += ipvlan/
> +obj-$(CONFIG_IPVTAP) += ipvlan/
> obj-$(CONFIG_DUMMY) += dummy.o
> obj-$(CONFIG_EQUALIZER) += eql.o
> obj-$(CONFIG_IFB) += ifb.o
> diff --git a/drivers/net/ipvlan/Makefile b/drivers/net/ipvlan/Makefile
> index df79910..8a2c64d 100644
> --- a/drivers/net/ipvlan/Makefile
> +++ b/drivers/net/ipvlan/Makefile
> @@ -3,5 +3,6 @@
> #
>
> obj-$(CONFIG_IPVLAN) += ipvlan.o
> +obj-$(CONFIG_IPVTAP) += ipvtap.o
>
> ipvlan-objs := ipvlan_core.o ipvlan_main.o
> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
> index dbfbb33..4362d88 100644
> --- a/drivers/net/ipvlan/ipvlan.h
> +++ b/drivers/net/ipvlan/ipvlan.h
> @@ -133,4 +133,11 @@ struct sk_buff *ipvlan_l3_rcv(struct net_device *dev, struct sk_buff *skb,
> u16 proto);
> unsigned int ipvlan_nf_input(void *priv, struct sk_buff *skb,
> const struct nf_hook_state *state);
> +void ipvlan_count_rx(const struct ipvl_dev *ipvlan,
> + unsigned int len, bool success, bool mcast);
> +int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[]);
> +void ipvlan_link_delete(struct net_device *dev, struct list_head *head);
> +void ipvlan_link_setup(struct net_device *dev);
> +int ipvlan_link_register(struct rtnl_link_ops *ops);
> #endif /* __IPVLAN_H */
> diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
> index 83ce74a..9af16ab 100644
> --- a/drivers/net/ipvlan/ipvlan_core.c
> +++ b/drivers/net/ipvlan/ipvlan_core.c
> @@ -16,8 +16,8 @@ void ipvlan_init_secret(void)
> net_get_random_once(&ipvlan_jhash_secret, sizeof(ipvlan_jhash_secret));
> }
>
> -static void ipvlan_count_rx(const struct ipvl_dev *ipvlan,
> - unsigned int len, bool success, bool mcast)
> +void ipvlan_count_rx(const struct ipvl_dev *ipvlan,
> + unsigned int len, bool success, bool mcast)
> {
> if (!ipvlan)
> return;
> @@ -36,6 +36,7 @@ static void ipvlan_count_rx(const struct ipvl_dev *ipvlan,
> this_cpu_inc(ipvlan->pcpu_stats->rx_errs);
> }
> }
> +EXPORT_SYMBOL_GPL(ipvlan_count_rx);
Why export, isn't just removing 'static' enough?
>
> static u8 ipvlan_get_v6_hash(const void *iaddr)
> {
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 8b0f993..666a05d 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -13,14 +13,14 @@ static u32 ipvl_nf_hook_refcnt = 0;
>
> static struct nf_hook_ops ipvl_nfops[] __read_mostly = {
> {
> - .hook = ipvlan_nf_input,
> - .pf = NFPROTO_IPV4,
> + .hook = ipvlan_nf_input,
> + .pf = NFPROTO_IPV4,
spurious?
> .hooknum = NF_INET_LOCAL_IN,
> .priority = INT_MAX,
> },
> {
> - .hook = ipvlan_nf_input,
> - .pf = NFPROTO_IPV6,
> + .hook = ipvlan_nf_input,
> + .pf = NFPROTO_IPV6,
spurious??
> .hooknum = NF_INET_LOCAL_IN,
> .priority = INT_MAX,
> },
> @@ -398,7 +398,7 @@ static int ipvlan_hard_header(struct sk_buff *skb, struct net_device *dev,
> }
>
> static const struct header_ops ipvlan_header_ops = {
> - .create = ipvlan_hard_header,
> + .create = ipvlan_hard_header,
spurious???
> .parse = eth_header_parse,
> .cache = eth_header_cache,
> .cache_update = eth_header_cache_update,
> @@ -494,8 +494,8 @@ static int ipvlan_nl_fillinfo(struct sk_buff *skb,
> return ret;
> }
>
> -static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> - struct nlattr *tb[], struct nlattr *data[])
> +int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[])
> {
> struct ipvl_dev *ipvlan = netdev_priv(dev);
> struct ipvl_port *port;
> @@ -567,8 +567,9 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> ipvlan_port_destroy(phy_dev);
> return err;
> }
> +EXPORT_SYMBOL_GPL(ipvlan_link_new);
>
> -static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
> +void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
> {
> struct ipvl_dev *ipvlan = netdev_priv(dev);
> struct ipvl_addr *addr, *next;
> @@ -583,8 +584,9 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
> unregister_netdevice_queue(dev, head);
> netdev_upper_dev_unlink(ipvlan->phy_dev, dev);
> }
> +EXPORT_SYMBOL_GPL(ipvlan_link_delete);
>
> -static void ipvlan_link_setup(struct net_device *dev)
> +void ipvlan_link_setup(struct net_device *dev)
> {
> ether_setup(dev);
>
> @@ -595,6 +597,7 @@ static void ipvlan_link_setup(struct net_device *dev)
> dev->header_ops = &ipvlan_header_ops;
> dev->ethtool_ops = &ipvlan_ethtool_ops;
> }
> +EXPORT_SYMBOL_GPL(ipvlan_link_setup);
>
> static const struct nla_policy ipvlan_nl_policy[IFLA_IPVLAN_MAX + 1] =
> {
> @@ -605,22 +608,22 @@ static struct rtnl_link_ops ipvlan_link_ops = {
> .kind = "ipvlan",
> .priv_size = sizeof(struct ipvl_dev),
>
> - .get_size = ipvlan_nl_getsize,
> - .policy = ipvlan_nl_policy,
> - .validate = ipvlan_nl_validate,
> - .fill_info = ipvlan_nl_fillinfo,
> - .changelink = ipvlan_nl_changelink,
> - .maxtype = IFLA_IPVLAN_MAX,
> -
> .setup = ipvlan_link_setup,
> .newlink = ipvlan_link_new,
> .dellink = ipvlan_link_delete,
> };
>
> -static int ipvlan_link_register(struct rtnl_link_ops *ops)
> +int ipvlan_link_register(struct rtnl_link_ops *ops)
> {
> + ops->get_size = ipvlan_nl_getsize;
> + ops->policy = ipvlan_nl_policy;
> + ops->validate = ipvlan_nl_validate;
> + ops->fill_info = ipvlan_nl_fillinfo;
> + ops->changelink = ipvlan_nl_changelink;
> + ops->maxtype = IFLA_IPVLAN_MAX;
> return rtnl_link_register(ops);
> }
> +EXPORT_SYMBOL_GPL(ipvlan_link_register);
>
> static int ipvlan_device_event(struct notifier_block *unused,
> unsigned long event, void *ptr)
> diff --git a/drivers/net/ipvlan/ipvtap.c b/drivers/net/ipvlan/ipvtap.c
> new file mode 100644
> index 0000000..0422cdf
> --- /dev/null
> +++ b/drivers/net/ipvlan/ipvtap.c
> @@ -0,0 +1,238 @@
> +#include <linux/etherdevice.h>
> +#include "ipvlan.h"
> +#include <linux/if_vlan.h>
> +#include <linux/if_tap.h>
> +#include <linux/interrupt.h>
> +#include <linux/nsproxy.h>
> +#include <linux/compat.h>
> +#include <linux/if_tun.h>
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/cache.h>
> +#include <linux/sched.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +#include <linux/cdev.h>
> +#include <linux/idr.h>
> +#include <linux/fs.h>
> +#include <linux/uio.h>
> +
> +#include <net/net_namespace.h>
> +#include <net/rtnetlink.h>
> +#include <net/sock.h>
> +#include <linux/virtio_net.h>
> +
> +#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> + NETIF_F_TSO6 | NETIF_F_UFO)
> +
> +static dev_t ipvtap_major;
> +static struct cdev ipvtap_cdev;
> +
> +static const void *ipvtap_net_namespace(struct device *d)
> +{
> + struct net_device *dev = to_net_dev(d->parent);
> + return dev_net(dev);
> +}
> +
> +static struct class ipvtap_class = {
> + .name = "ipvtap",
> + .owner = THIS_MODULE,
> + .ns_type = &net_ns_type_operations,
> + .namespace = ipvtap_net_namespace,
> +};
> +
> +struct ipvtap_dev {
> + struct ipvl_dev vlan;
> + struct tap_dev tap;
> +};
> +
> +static void ipvtap_count_tx_dropped(struct tap_dev *tap)
> +{
> + struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct ipvtap_dev, tap);
> +
> + this_cpu_inc(vlan->pcpu_stats->tx_drps);
> +}
> +
> +static void ipvtap_count_rx_dropped(struct tap_dev *tap)
> +{
> + struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct ipvtap_dev, tap);
> +
> + ipvlan_count_rx(vlan, 0, 0, 0);
> +}
> +
> +static void ipvtap_update_features(struct tap_dev *tap,
> + netdev_features_t features)
> +{
> + struct ipvl_dev *vlan = (struct ipvl_dev *)container_of(tap, struct ipvtap_dev, tap);
> +
> + vlan->sfeatures = features;
> + netdev_update_features(vlan->dev);
> +}
> +
> +static int ipvtap_newlink(struct net *src_net,
> + struct net_device *dev,
> + struct nlattr *tb[],
> + struct nlattr *data[])
> +{
> + struct ipvtap_dev *vlantap = netdev_priv(dev);
> + int err;
> +
> + INIT_LIST_HEAD(&vlantap->tap.queue_list);
> +
> + /* Since macvlan supports all offloads by default, make
> + * tap support all offloads also.
> + */
> + vlantap->tap.tap_features = TUN_OFFLOADS;
> + vlantap->tap.count_tx_dropped = ipvtap_count_tx_dropped;
> + vlantap->tap.update_features = ipvtap_update_features;
> + vlantap->tap.count_rx_dropped = ipvtap_count_rx_dropped;
> +
> + err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap);
> + if (err)
> + return err;
> +
> + /* Don't put anything that may fail after macvlan_common_newlink
> + * because we can't undo what it does.
> + */
> + err = ipvlan_link_new(src_net, dev, tb, data);
> + if (err) {
> + netdev_rx_handler_unregister(dev);
> + return err;
> + }
> +
> + vlantap->tap.dev = vlantap->vlan.dev;
> +
> + return err;
> +}
> +
> +static void ipvtap_dellink(struct net_device *dev,
> + struct list_head *head)
> +{
> + struct ipvtap_dev *vlan = netdev_priv(dev);
> +
> + netdev_rx_handler_unregister(dev);
> + tap_del_queues(&vlan->tap);
> + ipvlan_link_delete(dev, head);
> +}
> +
> +static void ipvtap_setup(struct net_device *dev)
> +{
> + ipvlan_link_setup(dev);
> + dev->tx_queue_len = TUN_READQ_SIZE;
> + dev->priv_flags &= ~IFF_NO_QUEUE;
> +}
> +
> +static struct rtnl_link_ops ipvtap_link_ops __read_mostly = {
> + .kind = "ipvtap",
> + .setup = ipvtap_setup,
> + .newlink = ipvtap_newlink,
> + .dellink = ipvtap_dellink,
> + .priv_size = sizeof(struct ipvtap_dev),
> +};
> +
> +static int ipvtap_device_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> + struct ipvtap_dev *vlantap;
> + struct device *classdev;
> + dev_t devt;
> + int err;
> + char tap_name[IFNAMSIZ];
> +
> + if (dev->rtnl_link_ops != &ipvtap_link_ops)
> + return NOTIFY_DONE;
> +
> + snprintf(tap_name, IFNAMSIZ, "tap%d", dev->ifindex);
> + vlantap = netdev_priv(dev);
> +
> + switch (event) {
> + case NETDEV_REGISTER:
> + /* Create the device node here after the network device has
> + * been registered but before register_netdevice has
> + * finished running.
> + */
> + err = tap_get_minor(ipvtap_major, &vlantap->tap);
> + if (err)
> + return notifier_from_errno(err);
> +
> + devt = MKDEV(MAJOR(ipvtap_major), vlantap->tap.minor);
> + classdev = device_create(&ipvtap_class, &dev->dev, devt,
> + dev, tap_name);
> + if (IS_ERR(classdev)) {
> + tap_free_minor(ipvtap_major, &vlantap->tap);
> + return notifier_from_errno(PTR_ERR(classdev));
> + }
> + err = sysfs_create_link(&dev->dev.kobj, &classdev->kobj,
> + tap_name);
> + if (err)
> + return notifier_from_errno(err);
> + break;
> + case NETDEV_UNREGISTER:
> + /* vlan->minor == 0 if NETDEV_REGISTER above failed */
> + if (vlantap->tap.minor == 0)
> + break;
> + sysfs_remove_link(&dev->dev.kobj, tap_name);
> + devt = MKDEV(MAJOR(ipvtap_major), vlantap->tap.minor);
> + device_destroy(&ipvtap_class, devt);
> + tap_free_minor(ipvtap_major, &vlantap->tap);
> + break;
> + case NETDEV_CHANGE_TX_QUEUE_LEN:
> + if (tap_queue_resize(&vlantap->tap))
> + return NOTIFY_BAD;
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block ipvtap_notifier_block __read_mostly = {
> + .notifier_call = ipvtap_device_event,
> +};
> +
> +static int ipvtap_init(void)
> +{
> + int err;
> +
> + err = tap_create_cdev(&ipvtap_cdev, &ipvtap_major, "ipvtap");
> +
> + if (err)
> + goto out1;
> +
> + err = class_register(&ipvtap_class);
> + if (err)
> + goto out2;
> +
> + err = register_netdevice_notifier(&ipvtap_notifier_block);
> + if (err)
> + goto out3;
> +
> + err = ipvlan_link_register(&ipvtap_link_ops);
> + if (err)
> + goto out4;
> +
> + return 0;
> +
> +out4:
> + unregister_netdevice_notifier(&ipvtap_notifier_block);
> +out3:
> + class_unregister(&ipvtap_class);
> +out2:
> + cdev_del(&ipvtap_cdev);
> +out1:
> + return err;
> +}
> +module_init(ipvtap_init);
> +
> +static void ipvtap_exit(void)
> +{
> + rtnl_link_unregister(&ipvtap_link_ops);
> + unregister_netdevice_notifier(&ipvtap_notifier_block);
> + class_unregister(&ipvtap_class);
> + tap_destroy_cdev(ipvtap_major, &ipvtap_cdev);
> +}
> +module_exit(ipvtap_exit);
> +MODULE_ALIAS_RTNL_LINK("ipvtap");
> +MODULE_AUTHOR("Arnd Bergmann <arnd@arndb.de>");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
^ permalink raw reply
* RE: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-06 23:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe@st.com, Jarod Wilson, Alexandre TORGUE,
Joachim Eastwood, Niklas Cassel, Johan Hovold, Pavel Machek,
lars.persson@axis.com, netdev, LKML
In-Reply-To: <CAHp75VdkGcQM_fW6LLuQ1JC67p4vGNWKcyxPs5Sc-4tFi2MGLg@mail.gmail.com>
> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Saturday, January 07, 2017 1:07 AM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> Cc: David S. Miller <davem@davemloft.net>; Joao Pinto
> <Joao.Pinto@synopsys.com>; Giuseppe CAVALLARO <peppe.cavallaro@st.com>;
> seraphin.bonnaffe@st.com; Jarod Wilson <jarod@redhat.com>; Alexandre
> TORGUE <alexandre.torgue@gmail.com>; Joachim Eastwood
> <manabian@gmail.com>; Niklas Cassel <niklas.cassel@axis.com>; Johan Hovold
> <johan@kernel.org>; Pavel Machek <pavel@ucw.cz>; lars.persson@axis.com;
> netdev <netdev@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid
> range
>
> On Sat, Jan 7, 2017 at 2:49 AM, Kweh, Hock Leong
> <hock.leong.kweh@intel.com> wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > There is no checking valid value of maxmtu when getting it from device tree.
> > This resolution added the checking condition to ensure the assignment is
> > made within a valid range.
>
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
> > ndev->max_mtu = JUMBO_LEN;
> > else
> > ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
> > - if (priv->plat->maxmtu < ndev->max_mtu)
>
> > +
>
> The lines are logically grouped here. No need to split it. Thus,
> remove this extra line.
>
Ok noted.
> > + if ((priv->plat->maxmtu < ndev->max_mtu) &&
> > + (priv->plat->maxmtu >= ndev->min_mtu))
>
> > ndev->max_mtu = priv->plat->maxmtu;
>
> > + else if (priv->plat->maxmtu < ndev->min_mtu)
>
> And if it > ndev->max_mtu?..
>
Base on my understanding to the original code, the "maxmtu >= ndev->max_mtu"
is meant for products that would want to use the value from logic which is just above
this statement where you just ask me not to add new line. That the reason the
stmmac_platform.c put in "plat->maxmtu = JUMBO_LEN;" as generic and I also
follow it in stmmac_pci.c.
Or do you mean only take maxmtu = JUMBO_LEN for the option to use driver itself
assignment statement above and all the > max_mtu consider invalid?
> > + netdev_warn(priv->dev,
> > + "%s: warning: maxmtu having invalid value (%d)\n",
> > + __func__, priv->plat->maxmtu);
>
>
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > @@ -204,6 +204,10 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
> >
> > pci_set_master(pdev);
> >
> > + /* Set the maxmtu to a default of JUMBO_LEN in case the
> > + * parameter is not defined for the device.
> > + */
> > + plat->maxmtu = JUMBO_LEN;
>
> Please, use *_default_data() hooks for that.
>
> At some point it might make sense to extract
> static int common_default_data() {...}
> and call it at the beginning of the rest of *_defautl_data() hooks.
>
Just try to understand, are you referring changing the code something
like this:
stmmac_default_data(plat);
if (info) {
info->pdev = pdev;
if (info->setup) {
ret = info->setup(plat, info);
if (ret)
return ret;
}
}
Where all the common code is inside the stmmac_default_data()?
Anyway, this patch is focus for fixing the maxmtu assignment in valid range.
I will submit V4 to put "plat->maxmtu = JUMBO_LEN;" into each *_default_data().
Regarding the common_default_data() would be a new patch for better code
structuring. Thanks.
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* [PATCH v4] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-07 8:10 UTC (permalink / raw)
To: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe, Jarod Wilson, Andy Shevchenko
Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
pavel, Kweh, Hock Leong, lars.persson, netdev, LKML
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
There is no checking valid value of maxmtu when getting it from device tree.
This resolution added the checking condition to ensure the assignment is
made within a valid range.
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
changelog v4:
* add print warning message when maxmtu > max_mtu as well
* add maxmtu = JUMBO_LEN into each *_default_data() at stmmac_pci.c
changelog v3:
* print the warning message only if maxmtu < min_mtu
* add maxmtu = JUMBO_LEN at stmmac_pci.c to follow stmmac_platform.c
changelog v2:
* correction of "devicetree" to "device tree" reported by Andy
* print warning message while maxmtu is not in valid range
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 6 ++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 92ac006..f2a352f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
ndev->max_mtu = JUMBO_LEN;
else
ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
- if (priv->plat->maxmtu < ndev->max_mtu)
+ if ((priv->plat->maxmtu < ndev->max_mtu) &&
+ (priv->plat->maxmtu >= ndev->min_mtu))
ndev->max_mtu = priv->plat->maxmtu;
+ else if ((priv->plat->maxmtu < ndev->min_mtu) ||
+ (priv->plat->maxmtu > ndev->max_mtu))
+ netdev_warn(priv->dev,
+ "%s: warning: maxmtu having invalid value (%d)\n",
+ __func__, priv->plat->maxmtu);
if (flow_ctrl)
priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a283177..3da4737 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -89,6 +89,9 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
/* Set default value for unicast filter entries */
plat->unicast_filter_entries = 1;
+
+ /* Set the maxmtu to a default of JUMBO_LEN */
+ plat->maxmtu = JUMBO_LEN;
}
static int quark_default_data(struct plat_stmmacenet_data *plat,
@@ -126,6 +129,9 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
/* Set default value for unicast filter entries */
plat->unicast_filter_entries = 1;
+ /* Set the maxmtu to a default of JUMBO_LEN */
+ plat->maxmtu = JUMBO_LEN;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid range
From: Andy Shevchenko @ 2017-01-07 0:07 UTC (permalink / raw)
To: Kweh, Hock Leong
Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe@st.com, Jarod Wilson, Alexandre TORGUE,
Joachim Eastwood, Niklas Cassel, Johan Hovold, Pavel Machek,
lars.persson@axis.com, netdev, LKML
In-Reply-To: <F54AEECA5E2B9541821D670476DAE19C5A918C87@PGSMSX102.gar.corp.intel.com>
On Sat, Jan 7, 2017 at 1:47 AM, Kweh, Hock Leong
<hock.leong.kweh@intel.com> wrote:
>> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> > @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
>> > ndev->max_mtu = JUMBO_LEN;
>> > else
>> > ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
>> > - if (priv->plat->maxmtu < ndev->max_mtu)
>>
>> > + if ((priv->plat->maxmtu < ndev->max_mtu) &&
>> > + (priv->plat->maxmtu >= ndev->min_mtu))
>>
>> > ndev->max_mtu = priv->plat->maxmtu;
>>
>> > + else if (priv->plat->maxmtu < ndev->min_mtu)
>>
>> And if it > ndev->max_mtu?..
>>
>
> Base on my understanding to the original code, the "maxmtu >= ndev->max_mtu"
> is meant for products that would want to use the value from logic which is just above
> this statement where you just ask me not to add new line. That the reason the
> stmmac_platform.c put in "plat->maxmtu = JUMBO_LEN;" as generic and I also
> follow it in stmmac_pci.c.
>
> Or do you mean only take maxmtu = JUMBO_LEN for the option to use driver itself
> assignment statement above and all the > max_mtu consider invalid?
So, just answer to the simple question: is it a valid case to have
plat->maxmtu > ndev->max_mtu? If it so, how is it used?
Otherwise we need a warning in such case. What did I miss?
>
>> > + netdev_warn(priv->dev,
>> > + "%s: warning: maxmtu having invalid value (%d)\n",
>> > + __func__, priv->plat->maxmtu);
>> > + /* Set the maxmtu to a default of JUMBO_LEN in case the
>> > + * parameter is not defined for the device.
>> > + */
>> > + plat->maxmtu = JUMBO_LEN;
>>
>> Please, use *_default_data() hooks for that.
>>
>> At some point it might make sense to extract
>> static int common_default_data() {...}
>> and call it at the beginning of the rest of *_defautl_data() hooks.
>>
>
> Just try to understand, are you referring changing the code something
> like this:
>
> stmmac_default_data(plat);
> if (info) {
> info->pdev = pdev;
> if (info->setup) {
> ret = info->setup(plat, info);
> if (ret)
> return ret;
> }
> }
>
> Where all the common code is inside the stmmac_default_data()?
No.
common_default_data()
{
... common defaults among *_default_data() ...
}
*_default_data()
{
...
common_default_data();
...
}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4] net: stmmac: fix maxmtu assignment to be within valid range
From: Andy Shevchenko @ 2017-01-07 0:12 UTC (permalink / raw)
To: Kweh, Hock Leong
Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe, Jarod Wilson, Alexandre TORGUE,
Joachim Eastwood, Niklas Cassel, Johan Hovold, Pavel Machek,
lars.persson, netdev, LKML
In-Reply-To: <1483776634-13095-1-git-send-email-hock.leong.kweh@intel.com>
On Sat, Jan 7, 2017 at 10:10 AM, Kweh, Hock Leong
<hock.leong.kweh@intel.com> wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>
> There is no checking valid value of maxmtu when getting it from device tree.
> This resolution added the checking condition to ensure the assignment is
> made within a valid range.
> changelog v4:
> * add print warning message when maxmtu > max_mtu as well
Yep.
> * add maxmtu = JUMBO_LEN into each *_default_data() at stmmac_pci.c
Yep.
But see comment below.
P.S. And perhaps next time send into our internal mailing list first for review.
> @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
> ndev->max_mtu = JUMBO_LEN;
> else
> ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
> - if (priv->plat->maxmtu < ndev->max_mtu)
> + if ((priv->plat->maxmtu < ndev->max_mtu) &&
> + (priv->plat->maxmtu >= ndev->min_mtu))
> ndev->max_mtu = priv->plat->maxmtu;
> + else if ((priv->plat->maxmtu < ndev->min_mtu) ||
> + (priv->plat->maxmtu > ndev->max_mtu))
> + netdev_warn(priv->dev,
What is the difference to just 'else'? (Returning back to my initial
proposal, I don't remember telling anything about 'else if' concept)
> + "%s: warning: maxmtu having invalid value (%d)\n",
> + __func__, priv->plat->maxmtu);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 02/12] net: ethernet: aquantia: Common functions and definitions
From: Lino Sanfilippo @ 2017-01-07 0:29 UTC (permalink / raw)
To: Alexander Loktionov, netdev, David VomLehn
Cc: Simon Edelhaus, Dmitrii Tarakanov, Pavel Belous
In-Reply-To: <41e24066f2a87cad644a902658846e109a6b406f.1483689029.git.vomlehn@texas.net>
Hi,
On 06.01.2017 09:06, Alexander Loktionov wrote:
> +
> +#define TXT(_T_) #_T_
> +#define TXTTXT(_T_) TXT(_T_)
do you really need these (IMHO ugly) macros? AFAICS you only use them to
build the driver
version string.
> +
> +#define AQ_CFG_DRV_AUTHOR "aQuantia"
> +#define AQ_CFG_DRV_DESC "aQuantia Corporation(R) Network Driver"
> +#define AQ_CFG_DRV_NAME "aquantia"
> +#define AQ_CFG_DRV_VERSION TXTTXT(NIC_MAJOR_DRIVER_VERSION)"."\
> + TXTTXT(NIC_MINOR_DRIVER_VERSION)"."\
> + TXTTXT(NIC_BUILD_DRIVER_VERSION)"."\
> + TXTTXT(NIC_REVISION_DRIVER_VERSION)
Regards,
Lino
^ permalink raw reply
* [PATCH next v1] ipvlan: don't use IDR for generating dev_id
From: Mahesh Bandewar @ 2017-01-07 0:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Mahesh Bandewar, Mahesh Bandewar, Eric Dumazet
From: Mahesh Bandewar <maheshb@google.com>
The patch 009146d117b ("ipvlan: assign unique dev-id for each slave
device.") used ida_simple_get() to generate dev_ids assigned to the
slave devices. However (Eric has pointed out that) there is a shortcoming
with that approach as it always uses the first available ID. This
becomes a problem when a slave gets deleted and a new slave gets added.
The ID gets reassigned causing the new slave to get the same link-local
address. This side-effect is undesirable.
This patch replaces IDR logic with a simple per-port variable that keeps
incrementing and wraps around when the MAX (0xFFFE) is reached. The
only downside is that this is an inefficient (n^2) search if there are
64k (or close to 64k) slaves in the system, the dev-id search takes time.
However having these many devices in the system has it's own challenges.
Fixes: 009146d117b ("ipvlan: assign unique dev-id for each slave device.")
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
CC: Eric Dumazet <edumazet@google.com>
---
drivers/net/ipvlan/ipvlan.h | 2 +-
drivers/net/ipvlan/ipvlan_main.c | 69 ++++++++++++++++++++++++++++++++++------
2 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 0a9068fdee0f..535f254324ba 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -94,10 +94,10 @@ struct ipvl_port {
struct hlist_head hlhead[IPVLAN_HASH_SIZE];
struct list_head ipvlans;
u16 mode;
+ u16 dev_id_base;
struct work_struct wq;
struct sk_buff_head backlog;
int count;
- struct ida ida;
};
struct ipvl_skb_cb {
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index ce7ca6a5aa8a..691662e02abb 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -119,7 +119,7 @@ static int ipvlan_port_create(struct net_device *dev)
skb_queue_head_init(&port->backlog);
INIT_WORK(&port->wq, ipvlan_process_multicast);
- ida_init(&port->ida);
+ port->dev_id_base = 1;
err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port);
if (err)
@@ -151,7 +151,6 @@ static void ipvlan_port_destroy(struct net_device *dev)
dev_put(skb->dev);
kfree_skb(skb);
}
- ida_destroy(&port->ida);
kfree(port);
}
@@ -496,6 +495,60 @@ static int ipvlan_nl_fillinfo(struct sk_buff *skb,
return ret;
}
+static u16 ipvlan_get_dev_id(struct ipvl_port *port)
+{
+ struct ipvl_dev *ipvlan;
+ u16 tid, dev_id = 0;
+ bool found, exhausted = false, second_round = false;
+
+ /* Idea here is to get the next available ID while avoiding
+ * reuse of already used IDs. Per port variable (dev_id_base)
+ * is used to get the next available ID.
+ *
+ * Possibilities are from 0x1 to 0xfffe so finding next available
+ * ID is not be that difficult. However if the system is up for a
+ * long time and these slave devices are getting added / deleted
+ * routinely and it reaches the MAX, then only the assignment will
+ * attempt to recycle previously used IDs starting from 0x1 (Provided
+ * these are not currently in use).
+ *
+ * In case of a failure, dev-id search will return 0. All IDs in-use
+ * case is very inefficient (n^2 search) but having close to 64k
+ * slaves has it's own system limitations.
+ */
+ while (!exhausted) {
+ tid = port->dev_id_base++;
+
+ if (tid == 0xFFFE) {
+ /* Reached MAX; restart from 1 */
+ port->dev_id_base = 1;
+ if (second_round)
+ exhausted = true;
+ else
+ second_round = true;
+ continue;
+ }
+ /* Should not be same as the masters' (if present) */
+ if (tid == port->dev->dev_id)
+ continue;
+ /* Make sure that it's already not assigned to any of the
+ * existing slaves.
+ */
+ found = false;
+ list_for_each_entry(ipvlan, &port->ipvlans, pnode) {
+ if (ipvlan->dev->dev_id == tid) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ dev_id = tid;
+ break;
+ }
+ }
+ return dev_id;
+}
+
static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
@@ -540,10 +593,11 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
* Assign IDs between 0x1 and 0xFFFE (used by the master) to each
* slave link [see addrconf_ifid_eui48()].
*/
- err = ida_simple_get(&port->ida, 1, 0xFFFE, GFP_KERNEL);
- if (err < 0)
+ dev->dev_id = ipvlan_get_dev_id(port);
+ if (dev->dev_id == 0) {
+ err = -ENOSPC;
goto destroy_ipvlan_port;
- dev->dev_id = err;
+ }
/* TODO Probably put random address here to be presented to the
* world but keep using the physical-dev address for the outgoing
@@ -555,7 +609,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
err = register_netdevice(dev);
if (err < 0)
- goto remove_ida;
+ goto destroy_ipvlan_port;
err = netdev_upper_dev_link(phy_dev, dev);
if (err) {
@@ -574,8 +628,6 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
netdev_upper_dev_unlink(phy_dev, dev);
unregister_netdev:
unregister_netdevice(dev);
-remove_ida:
- ida_simple_remove(&port->ida, dev->dev_id);
destroy_ipvlan_port:
if (create)
ipvlan_port_destroy(phy_dev);
@@ -593,7 +645,6 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
kfree_rcu(addr, rcu);
}
- ida_simple_remove(&ipvlan->port->ida, dev->dev_id);
list_del_rcu(&ipvlan->pnode);
unregister_netdevice_queue(dev, head);
netdev_upper_dev_unlink(ipvlan->phy_dev, dev);
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* RE: [PATCH v4] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-07 0:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe@st.com, Jarod Wilson, Alexandre TORGUE,
Joachim Eastwood, Niklas Cassel, Johan Hovold, Pavel Machek,
lars.persson@axis.com, netdev, LKML
In-Reply-To: <CAHp75VeBJsTR9OrdYQqs7fCb40KzyByehX97VdKm8CerDUwVqg@mail.gmail.com>
> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Saturday, January 07, 2017 8:12 AM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> Cc: David S. Miller <davem@davemloft.net>; Joao Pinto
> <Joao.Pinto@synopsys.com>; Giuseppe CAVALLARO <peppe.cavallaro@st.com>;
> seraphin.bonnaffe@st.com; Jarod Wilson <jarod@redhat.com>; Alexandre
> TORGUE <alexandre.torgue@gmail.com>; Joachim Eastwood
> <manabian@gmail.com>; Niklas Cassel <niklas.cassel@axis.com>; Johan Hovold
> <johan@kernel.org>; Pavel Machek <pavel@ucw.cz>; lars.persson@axis.com;
> netdev <netdev@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v4] net: stmmac: fix maxmtu assignment to be within valid
> range
>
> On Sat, Jan 7, 2017 at 10:10 AM, Kweh, Hock Leong
> <hock.leong.kweh@intel.com> wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > There is no checking valid value of maxmtu when getting it from device tree.
> > This resolution added the checking condition to ensure the assignment is
> > made within a valid range.
>
> > changelog v4:
> > * add print warning message when maxmtu > max_mtu as well
>
> Yep.
>
> > * add maxmtu = JUMBO_LEN into each *_default_data() at stmmac_pci.c
>
> Yep.
>
> But see comment below.
>
> P.S. And perhaps next time send into our internal mailing list first for review.
>
> > @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
> > ndev->max_mtu = JUMBO_LEN;
> > else
> > ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
> > - if (priv->plat->maxmtu < ndev->max_mtu)
> > + if ((priv->plat->maxmtu < ndev->max_mtu) &&
> > + (priv->plat->maxmtu >= ndev->min_mtu))
> > ndev->max_mtu = priv->plat->maxmtu;
>
> > + else if ((priv->plat->maxmtu < ndev->min_mtu) ||
> > + (priv->plat->maxmtu > ndev->max_mtu))
> > + netdev_warn(priv->dev,
>
> What is the difference to just 'else'? (Returning back to my initial
> proposal, I don't remember telling anything about 'else if' concept)
>
When priv->plat->maxmtu == ndev->max_mtu will not be a warning message.
Oh NO ... it is a valid case for priv->plat->maxmtu > ndev->max_mtu if the
assignment of ndev->max_mtu is using SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN),
which is < JUMBO_LEN, then priv->plat->maxmtu > ndev->max_mtu is valid.
Revert back and submit V5. Thanks.
> > + "%s: warning: maxmtu having invalid value (%d)\n",
> > + __func__, priv->plat->maxmtu);
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* RE: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-07 0:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe@st.com, Jarod Wilson, Alexandre TORGUE,
Joachim Eastwood, Niklas Cassel, Johan Hovold, Pavel Machek,
lars.persson@axis.com, netdev, LKML
In-Reply-To: <CAHp75VeWs8LXS+zaCht8reLMLRtz+583K91QvoPquuUxC-rwWg@mail.gmail.com>
> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Saturday, January 07, 2017 8:07 AM
> To: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> Cc: David S. Miller <davem@davemloft.net>; Joao Pinto
> <Joao.Pinto@synopsys.com>; Giuseppe CAVALLARO <peppe.cavallaro@st.com>;
> seraphin.bonnaffe@st.com; Jarod Wilson <jarod@redhat.com>; Alexandre
> TORGUE <alexandre.torgue@gmail.com>; Joachim Eastwood
> <manabian@gmail.com>; Niklas Cassel <niklas.cassel@axis.com>; Johan Hovold
> <johan@kernel.org>; Pavel Machek <pavel@ucw.cz>; lars.persson@axis.com;
> netdev <netdev@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v3] net: stmmac: fix maxmtu assignment to be within valid
> range
>
> On Sat, Jan 7, 2017 at 1:47 AM, Kweh, Hock Leong
> <hock.leong.kweh@intel.com> wrote:
>
> >> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> > @@ -3345,8 +3345,14 @@ int stmmac_dvr_probe(struct device *device,
> >> > ndev->max_mtu = JUMBO_LEN;
> >> > else
> >> > ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD +
> NET_IP_ALIGN);
> >> > - if (priv->plat->maxmtu < ndev->max_mtu)
> >>
> >> > + if ((priv->plat->maxmtu < ndev->max_mtu) &&
> >> > + (priv->plat->maxmtu >= ndev->min_mtu))
> >>
> >> > ndev->max_mtu = priv->plat->maxmtu;
> >>
> >> > + else if (priv->plat->maxmtu < ndev->min_mtu)
> >>
> >> And if it > ndev->max_mtu?..
> >>
> >
> > Base on my understanding to the original code, the "maxmtu >= ndev-
> >max_mtu"
> > is meant for products that would want to use the value from logic which is just
> above
> > this statement where you just ask me not to add new line. That the reason the
> > stmmac_platform.c put in "plat->maxmtu = JUMBO_LEN;" as generic and I
> also
> > follow it in stmmac_pci.c.
> >
> > Or do you mean only take maxmtu = JUMBO_LEN for the option to use driver
> itself
> > assignment statement above and all the > max_mtu consider invalid?
>
> So, just answer to the simple question: is it a valid case to have
> plat->maxmtu > ndev->max_mtu? If it so, how is it used?
> Otherwise we need a warning in such case. What did I miss?
>
it is a valid case for priv->plat->maxmtu > ndev->max_mtu if referring
to the statement above it:
/* MTU range: 46 - hw-specific max */
ndev->min_mtu = ETH_ZLEN - ETH_HLEN;
if ((priv->plat->enh_desc) || (priv->synopsys_id >= DWMAC_CORE_4_00))
ndev->max_mtu = JUMBO_LEN;
else
ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
When the ndev->max_mtu go into the else statement, then the assignment in
stmmac_platform.c & stammac_pci.c plat->maxmtu = JUMBO_LEN is actually
greater than ndev->max_mtu. That is what I understanding that maxmtu > max_mtu
is an option trick to allow driver assign value through the logic above instead of getting
it from of_property_read_u32(np, "max-frame-size", &plat->maxmtu); or *_default_data().
I need to revert back the V4 and submit V5.
> >
> >> > + netdev_warn(priv->dev,
> >> > + "%s: warning: maxmtu having invalid value (%d)\n",
> >> > + __func__, priv->plat->maxmtu);
>
>
> >> > + /* Set the maxmtu to a default of JUMBO_LEN in case the
> >> > + * parameter is not defined for the device.
> >> > + */
> >> > + plat->maxmtu = JUMBO_LEN;
> >>
> >> Please, use *_default_data() hooks for that.
> >>
> >> At some point it might make sense to extract
> >> static int common_default_data() {...}
> >> and call it at the beginning of the rest of *_defautl_data() hooks.
> >>
> >
> > Just try to understand, are you referring changing the code something
> > like this:
> >
> > stmmac_default_data(plat);
> > if (info) {
> > info->pdev = pdev;
> > if (info->setup) {
> > ret = info->setup(plat, info);
> > if (ret)
> > return ret;
> > }
> > }
> >
> > Where all the common code is inside the stmmac_default_data()?
>
> No.
>
> common_default_data()
> {
> ... common defaults among *_default_data() ...
> }
>
> *_default_data()
> {
> ...
> common_default_data();
> ...
> }
>
Ok noted. Will be a separate patch. Thanks.
Regards,
Wilson
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* [PATCH net-next] liquidio: store the L4 hash of rx packets in skb
From: Felix Manlunas @ 2017-01-07 0:55 UTC (permalink / raw)
To: davem; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
From: Prasad Kanneganti <prasad.kanneganti@cavium.com>
Store the L4 hash of received packets in the skb; the hash is computed in
the NIC firmware.
Signed-off-by: Prasad Kanneganti <prasad.kanneganti@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 17 +++++++++++++++--
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 15 ++++++++++++++-
drivers/net/ethernet/cavium/liquidio/liquidio_common.h | 2 ++
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 39a9665..adae858 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -2263,6 +2263,7 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)),
struct skb_shared_hwtstamps *shhwtstamps;
u64 ns;
u16 vtag = 0;
+ u32 r_dh_off;
struct net_device *netdev = (struct net_device *)arg;
struct octeon_droq *droq = container_of(param, struct octeon_droq,
napi);
@@ -2308,6 +2309,8 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)),
put_page(pg_info->page);
}
+ r_dh_off = (rh->r_dh.len - 1) * BYTES_PER_DHLEN_UNIT;
+
if (((oct->chip_id == OCTEON_CN66XX) ||
(oct->chip_id == OCTEON_CN68XX)) &&
ptp_enable) {
@@ -2320,16 +2323,26 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)),
/* Nanoseconds are in the first 64-bits
* of the packet.
*/
- memcpy(&ns, (skb->data), sizeof(ns));
+ memcpy(&ns, (skb->data + r_dh_off),
+ sizeof(ns));
+ r_dh_off -= BYTES_PER_DHLEN_UNIT;
shhwtstamps = skb_hwtstamps(skb);
shhwtstamps->hwtstamp =
ns_to_ktime(ns +
lio->ptp_adjust);
}
- skb_pull(skb, sizeof(ns));
}
}
+ if (rh->r_dh.has_hash) {
+ u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off));
+
+ skb_set_hash(skb, hash, PKT_HASH_TYPE_L4);
+ r_dh_off -= BYTES_PER_DHLEN_UNIT;
+ }
+
+ skb_pull(skb, rh->r_dh.len * BYTES_PER_DHLEN_UNIT);
+
skb->protocol = eth_type_trans(skb, skb->dev);
if ((netdev->features & NETIF_F_RXCSUM) &&
(((rh->r_dh.encap_on) &&
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 70d96c1..c0ee746 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -1497,6 +1497,7 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)),
struct net_device *netdev = (struct net_device *)arg;
struct sk_buff *skb = (struct sk_buff *)skbuff;
u16 vtag = 0;
+ u32 r_dh_off;
if (netdev) {
struct lio *lio = GET_LIO(netdev);
@@ -1540,7 +1541,19 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)),
put_page(pg_info->page);
}
- skb_pull(skb, rh->r_dh.len * 8);
+ r_dh_off = (rh->r_dh.len - 1) * BYTES_PER_DHLEN_UNIT;
+
+ if (rh->r_dh.has_hwtstamp)
+ r_dh_off -= BYTES_PER_DHLEN_UNIT;
+
+ if (rh->r_dh.has_hash) {
+ u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off));
+
+ skb_set_hash(skb, hash, PKT_HASH_TYPE_L4);
+ r_dh_off -= BYTES_PER_DHLEN_UNIT;
+ }
+
+ skb_pull(skb, rh->r_dh.len * BYTES_PER_DHLEN_UNIT);
skb->protocol = eth_type_trans(skb, skb->dev);
if ((netdev->features & NETIF_F_RXCSUM) &&
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
index ba329f6..bc0af8a 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
@@ -98,6 +98,8 @@ enum octeon_tag_type {
#define CVM_DRV_INVALID_APP (CVM_DRV_APP_START + 0x2)
#define CVM_DRV_APP_END (CVM_DRV_INVALID_APP - 1)
+#define BYTES_PER_DHLEN_UNIT 8
+
static inline u32 incr_index(u32 index, u32 count, u32 max)
{
if ((index + count) >= max)
^ permalink raw reply related
* Re: [PATCH v2 03/12] net: ethernet: aquantia: Add ring support code
From: Lino Sanfilippo @ 2017-01-07 1:05 UTC (permalink / raw)
To: Alexander Loktionov, netdev, David VomLehn
Cc: Simon Edelhaus, Dmitrii Tarakanov, Pavel Belous
In-Reply-To: <7d0b8bdb9c3ec1d2cbfb136796dbfc66e0ab535d.1483689029.git.vomlehn@texas.net>
On 06.01.2017 09:06, Alexander Loktionov wrote:
> From: David VomLehn <vomlehn@texas.net>
>
> Add code to support the transmit and receive ring buffers.
>
> Signed-off-by: Alexander Loktionov <Alexander.Loktionov@aquantia.com>
> Signed-off-by: Dmitrii Tarakanov <Dmitrii.Tarakanov@aquantia.com>
> Signed-off-by: Pavel Belous <Pavel.Belous@aquantia.com>
> Signed-off-by: David M. VomLehn <vomlehn@texas.net>
> ---
> drivers/net/ethernet/aquantia/aq_ring.c | 380 ++++++++++++++++++++++++++++++++
> drivers/net/ethernet/aquantia/aq_ring.h | 147 ++++++++++++
> 2 files changed, 527 insertions(+)
> create mode 100644 drivers/net/ethernet/aquantia/aq_ring.c
> create mode 100644 drivers/net/ethernet/aquantia/aq_ring.h
>
> diff --git a/drivers/net/ethernet/aquantia/aq_ring.c b/drivers/net/ethernet/aquantia/aq_ring.c
> new file mode 100644
> index 0000000..a7ef6aa
> --- /dev/null
> +++ b/drivers/net/ethernet/aquantia/aq_ring.c
> @@ -0,0 +1,380 @@
> +/*
> + * aQuantia Corporation Network Driver
> + * Copyright (C) 2014-2016 aQuantia Corporation. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +/* File aq_pci_ring.c: Definition of functions for Rx/Tx rings. */
> +
> +#include "aq_ring.h"
> +#include "aq_nic.h"
> +#include "aq_hw.h"
> +
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +
> +static struct aq_ring_s *aq_ring_alloc(struct aq_ring_s *self,
> + struct aq_nic_s *aq_nic,
> + struct aq_nic_cfg_s *aq_nic_cfg)
> +{
> + int err = 0;
> +
> + if (!self) {
> + err = -ENOMEM;
> + goto err_exit;
> + }
Why do you check this parameter but not the others?
Beside this since the function is static the only caller is your own
code, so why is a check
needed at all? Can self ever be NULL? Same pattern occurs in several
other functions.
> +
> +struct aq_ring_s *aq_ring_tx_alloc(struct aq_ring_s *self,
> + struct aq_nic_s *aq_nic,
> + unsigned int idx,
> + struct aq_nic_cfg_s *aq_nic_cfg)
>
> +
> +struct aq_ring_s *aq_ring_rx_alloc(struct aq_ring_s *self,
> + struct aq_nic_s *aq_nic,
> + unsigned int idx,
> + struct aq_nic_cfg_s *aq_nic_cfg)
> +{
> + int err = 0;
> +
> + if (!self) {
> + err = -ENOMEM;
> + goto err_exit;
>
> +
> +void aq_ring_free(struct aq_ring_s *self)
> +{
> + if (!self)
> + goto err_exit;
> +
> + kfree(self->buff_ring);
> +
> + if (self->dx_ring)
> + dma_free_coherent(aq_nic_get_dev(self->aq_nic),
> + self->size * self->dx_size, self->dx_ring,
> + self->dx_ring_pa);
> +
> +err_exit:;
> +}
This does not make sense. Just return immediately if "self" is NULL (and
if you really have to make
this check).
> +
> +int aq_ring_rx_clean(struct aq_ring_s *self, int *work_done, int budget)
> +{
> + struct net_device *ndev = aq_nic_get_ndev(self->aq_nic);
> + int err = 0;
> + bool is_rsc_completed = true;
> +
> + for (; (self->sw_head != self->hw_head) && budget;
> + self->sw_head = aq_ring_next_dx(self, self->sw_head),
> + --budget, ++(*work_done)) {
> + struct aq_ring_buff_s *buff = &self->buff_ring[self->sw_head];
> + struct sk_buff *skb = NULL;
> + unsigned int next_ = 0U;
> + unsigned int i = 0U;
> + struct aq_ring_buff_s *buff_ = NULL;
> +
> + if (buff->is_error) {
> + __free_pages(buff->page, 0);
> + continue;
> + }
> +
> + if (buff->is_cleaned)
> + continue;
> +
> + ++self->stats.rx_packets;
> + ++ndev->stats.rx_packets;
> + ndev->stats.rx_bytes += buff->len;
> +
> + if (!buff->is_eop) {
> + for (next_ = buff->next,
> + buff_ = &self->buff_ring[next_]; true;
> + next_ = buff_->next,
> + buff_ = &self->buff_ring[next_]) {
> + is_rsc_completed =
> + aq_ring_dx_in_range(self->sw_head,
> + next_,
> + self->hw_head);
> +
> + if (unlikely(!is_rsc_completed)) {
> + is_rsc_completed = false;
> + break;
> + }
> +
> + if (buff_->is_eop)
> + break;
> + }
> +
> + if (!is_rsc_completed) {
> + err = 0;
> + goto err_exit;
> + }
> + }
> +
> + skb = netdev_alloc_skb(ndev, ETH_HLEN + AQ_CFG_IP_ALIGN);
> +
> + skb_reserve(skb, AQ_CFG_IP_ALIGN);
netdev_alloc_skb_ip_align() will do the proper alignment for you (BTW.
AQ_CFG_IP_ALIGN is defined as 0 in aq_cfg.h.
Is this on purpose?)
> + skb_put(skb, ETH_HLEN);
> + memcpy(skb->data, page_address(buff->page), ETH_HLEN);
> +
> + skb_add_rx_frag(skb, 0, buff->page, ETH_HLEN,
> + buff->len - ETH_HLEN,
> + SKB_TRUESIZE(buff->len - ETH_HLEN));
> + if (!buff->is_eop) {
> + for (i = 1U, next_ = buff->next,
> + buff_ = &self->buff_ring[next_]; true;
> + next_ = buff_->next,
> + buff_ = &self->buff_ring[next_], ++i) {
> + skb_add_rx_frag(skb, i, buff_->page, 0,
> + buff_->len,
> + SKB_TRUESIZE(buff->len -
> + ETH_HLEN));
> + buff_->is_cleaned = 1;
> +
> + if (buff_->is_eop)
> + break;
> + }
> + }
> +
> + skb->dev = ndev;
This is unnecessary. The following call of eth_type_trans() will do
this already.
Regards,
Lino
^ permalink raw reply
* Re: [next PATCH 00/11] ixgbe: Add support for writable pages and build_skb
From: Jeff Kirsher @ 2017-01-07 1:12 UTC (permalink / raw)
To: David Miller, alexander.duyck; +Cc: intel-wired-lan, netdev
In-Reply-To: <20170106.164116.1871412141301427533.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 669 bytes --]
On Fri, 2017-01-06 at 16:41 -0500, David Miller wrote:
> From: Alexander Duyck <alexander.duyck@gmail.com>
> Date: Fri, 06 Jan 2017 08:06:16 -0800
>
> > The testing matrix for all of these patches is going to be pretty
> > extensive. Basically we want to test these patches on as many
> > platforms
> > and architectures as possible with as many features being toggled as
> > possible including RSC, FCoE, SR-IOV, and Jumbo Frames all while
> > receiving
> > traffic.
>
> Overall looks very nice to me.
>
> I am assuming that I will get a formal submission once the necessary
> amount of testing is performed.
Yes, your assumption is correct. :-)
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net-next] liquidio: simplify octeon_flush_iq()
From: Felix Manlunas @ 2017-01-07 1:16 UTC (permalink / raw)
To: davem; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
From: Derek Chickles <derek.chickles@cavium.com>
Because every call to octeon_flush_iq() has a hardcoded 1 for the
pending_thresh argument, simplify that function by removing that argument.
This avoids one atomic read as well.
Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 2 +-
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 2 +-
drivers/net/ethernet/cavium/liquidio/octeon_iq.h | 2 +-
.../net/ethernet/cavium/liquidio/request_manager.c | 49 +++++++++++-----------
4 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 6443bc1..13f67a3 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -2441,7 +2441,7 @@ static int liquidio_napi_poll(struct napi_struct *napi, int budget)
iq = oct->instr_queue[iq_no];
if (iq) {
/* Process iq buffers with in the budget limits */
- tx_done = octeon_flush_iq(oct, iq, 1, budget);
+ tx_done = octeon_flush_iq(oct, iq, budget);
/* Update iq read-index rather than waiting for next interrupt.
* Return back if tx_done is false.
*/
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 70d96c1..55846f2 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -1627,7 +1627,7 @@ static int liquidio_napi_poll(struct napi_struct *napi, int budget)
iq = oct->instr_queue[iq_no];
if (iq) {
/* Process iq buffers with in the budget limits */
- tx_done = octeon_flush_iq(oct, iq, 1, budget);
+ tx_done = octeon_flush_iq(oct, iq, budget);
/* Update iq read-index rather than waiting for next interrupt.
* Return back if tx_done is false.
*/
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_iq.h b/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
index e04ca8f..4608a5a 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_iq.h
@@ -369,5 +369,5 @@ int octeon_setup_iq(struct octeon_device *oct, int ifidx,
void *app_ctx);
int
octeon_flush_iq(struct octeon_device *oct, struct octeon_instr_queue *iq,
- u32 pending_thresh, u32 napi_budget);
+ u32 napi_budget);
#endif /* __OCTEON_IQ_H__ */
diff --git a/drivers/net/ethernet/cavium/liquidio/request_manager.c b/drivers/net/ethernet/cavium/liquidio/request_manager.c
index 3ce6675..707bc15 100644
--- a/drivers/net/ethernet/cavium/liquidio/request_manager.c
+++ b/drivers/net/ethernet/cavium/liquidio/request_manager.c
@@ -455,7 +455,7 @@ lio_process_iq_request_list(struct octeon_device *oct,
/* Can only be called from process context */
int
octeon_flush_iq(struct octeon_device *oct, struct octeon_instr_queue *iq,
- u32 pending_thresh, u32 napi_budget)
+ u32 napi_budget)
{
u32 inst_processed = 0;
u32 tot_inst_processed = 0;
@@ -468,33 +468,32 @@ octeon_flush_iq(struct octeon_device *oct, struct octeon_instr_queue *iq,
iq->octeon_read_index = oct->fn_list.update_iq_read_idx(iq);
- if (atomic_read(&iq->instr_pending) >= (s32)pending_thresh) {
- do {
- /* Process any outstanding IQ packets. */
- if (iq->flush_index == iq->octeon_read_index)
- break;
-
- if (napi_budget)
- inst_processed = lio_process_iq_request_list
- (oct, iq,
- napi_budget - tot_inst_processed);
- else
- inst_processed =
- lio_process_iq_request_list(oct, iq, 0);
+ do {
+ /* Process any outstanding IQ packets. */
+ if (iq->flush_index == iq->octeon_read_index)
+ break;
- if (inst_processed) {
- atomic_sub(inst_processed, &iq->instr_pending);
- iq->stats.instr_processed += inst_processed;
- }
+ if (napi_budget)
+ inst_processed =
+ lio_process_iq_request_list(oct, iq,
+ napi_budget -
+ tot_inst_processed);
+ else
+ inst_processed =
+ lio_process_iq_request_list(oct, iq, 0);
+
+ if (inst_processed) {
+ atomic_sub(inst_processed, &iq->instr_pending);
+ iq->stats.instr_processed += inst_processed;
+ }
- tot_inst_processed += inst_processed;
- inst_processed = 0;
+ tot_inst_processed += inst_processed;
+ inst_processed = 0;
- } while (tot_inst_processed < napi_budget);
+ } while (tot_inst_processed < napi_budget);
- if (napi_budget && (tot_inst_processed >= napi_budget))
- tx_done = 0;
- }
+ if (napi_budget && (tot_inst_processed >= napi_budget))
+ tx_done = 0;
iq->last_db_time = jiffies;
@@ -530,7 +529,7 @@ static void __check_db_timeout(struct octeon_device *oct, u64 iq_no)
iq->last_db_time = jiffies;
/* Flush the instruction queue */
- octeon_flush_iq(oct, iq, 1, 0);
+ octeon_flush_iq(oct, iq, 0);
lio_enable_irq(NULL, iq);
}
^ permalink raw reply related
* [PATCH v5] net: stmmac: fix maxmtu assignment to be within valid range
From: Kweh, Hock Leong @ 2017-01-07 9:32 UTC (permalink / raw)
To: David S. Miller, Joao Pinto, Giuseppe CAVALLARO,
seraphin.bonnaffe, Jarod Wilson, Andy Shevchenko
Cc: Alexandre TORGUE, Joachim Eastwood, Niklas Cassel, Johan Hovold,
pavel, Kweh, Hock Leong, lars.persson, netdev, LKML
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
There is no checking valid value of maxmtu when getting it from
device tree. This resolution added the checking condition to
ensure the assignment is made within a valid range.
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
changelog v5:
* revert back that plat->maxmtu > ndev->max_mtu is a valid case
when ndev->max_mtu assignment is entering to the else statement
* add comment to enchance clarification
changelog v4:
* add print warning message when maxmtu > max_mtu as well
* add maxmtu = JUMBO_LEN into each *_default_data() at stmmac_pci.c
changelog v3:
* print the warning message only if maxmtu < min_mtu
* add maxmtu = JUMBO_LEN at stmmac_pci.c to follow stmmac_platform.c
changelog v2:
* correction of "devicetree" to "device tree" reported by Andy
* print warning message while maxmtu is not in valid range
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++++++-
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 6 ++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 92ac006..8e56dc4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3345,8 +3345,16 @@ int stmmac_dvr_probe(struct device *device,
ndev->max_mtu = JUMBO_LEN;
else
ndev->max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
- if (priv->plat->maxmtu < ndev->max_mtu)
+ /* Will not overwrite ndev->max_mtu if plat->maxmtu > ndev->max_mtu
+ * as well as plat->maxmtu < ndev->min_mtu which is a invalid range.
+ */
+ if ((priv->plat->maxmtu < ndev->max_mtu) &&
+ (priv->plat->maxmtu >= ndev->min_mtu))
ndev->max_mtu = priv->plat->maxmtu;
+ else if (priv->plat->maxmtu < ndev->min_mtu)
+ netdev_warn(priv->dev,
+ "%s: warning: maxmtu having invalid value (%d)\n",
+ __func__, priv->plat->maxmtu);
if (flow_ctrl)
priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a283177..3da4737 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -89,6 +89,9 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
/* Set default value for unicast filter entries */
plat->unicast_filter_entries = 1;
+
+ /* Set the maxmtu to a default of JUMBO_LEN */
+ plat->maxmtu = JUMBO_LEN;
}
static int quark_default_data(struct plat_stmmacenet_data *plat,
@@ -126,6 +129,9 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
/* Set default value for unicast filter entries */
plat->unicast_filter_entries = 1;
+ /* Set the maxmtu to a default of JUMBO_LEN */
+ plat->maxmtu = JUMBO_LEN;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions
From: David Miller @ 2017-01-07 1:38 UTC (permalink / raw)
To: phil; +Cc: netdev
In-Reply-To: <20170105190913.29986-1-phil@nwl.cc>
From: Phil Sutter <phil@nwl.cc>
Date: Thu, 5 Jan 2017 20:09:10 +0100
> This series adds VF support to dummy device driver after adding the
> necessary infrastructure changes:
>
> Patch 1 adds a netdevice callback for device-specific VF count
> retrieval. Patch 2 then changes dev_num_vf() implementation to make use
> of that new callback (if implemented), falling back to the old
> behaviour. Patch 3 then implements VF support in dummy, without the fake
> PCI parent device hack from v5.
Please don't make this a netdev specific method and interface.
Put the method in "struct bus_device", thereby making it a generic
"device" layer thing.
So the pci BUS type will implement pci_bus_type.num_vf(). And you'll
make a bus type for the dummy device to attach to which will implement
it's own.
^ permalink raw reply
* [PATCH net-next] net: ipmr: Remove nowait arg to ipmr_get_route
From: David Ahern @ 2017-01-07 1:39 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
ipmr_get_route has 1 caller and the nowait arg is 0. Remove the arg and
simplify ipmr_get_route accordingly.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
include/linux/mroute.h | 2 +-
net/ipv4/ipmr.c | 7 +------
net/ipv4/route.c | 2 +-
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/include/linux/mroute.h b/include/linux/mroute.h
index e5fb81376e92..f019b62f27b5 100644
--- a/include/linux/mroute.h
+++ b/include/linux/mroute.h
@@ -120,5 +120,5 @@ struct mfc_cache {
struct rtmsg;
int ipmr_get_route(struct net *net, struct sk_buff *skb,
__be32 saddr, __be32 daddr,
- struct rtmsg *rtm, int nowait, u32 portid);
+ struct rtmsg *rtm, u32 portid);
#endif
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index b35dda57586b..824c4fdf21eb 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2136,7 +2136,7 @@ static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
int ipmr_get_route(struct net *net, struct sk_buff *skb,
__be32 saddr, __be32 daddr,
- struct rtmsg *rtm, int nowait, u32 portid)
+ struct rtmsg *rtm, u32 portid)
{
struct mfc_cache *cache;
struct mr_table *mrt;
@@ -2160,11 +2160,6 @@ int ipmr_get_route(struct net *net, struct sk_buff *skb,
struct net_device *dev;
int vif = -1;
- if (nowait) {
- rcu_read_unlock();
- return -EAGAIN;
- }
-
dev = skb->dev;
read_lock(&mrt_lock);
if (dev)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index b9bff78274bf..f51823dc998b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2540,7 +2540,7 @@ static int rt_fill_info(struct net *net, __be32 dst, __be32 src, u32 table_id,
IPV4_DEVCONF_ALL(net, MC_FORWARDING)) {
int err = ipmr_get_route(net, skb,
fl4->saddr, fl4->daddr,
- r, 0, portid);
+ r, portid);
if (err <= 0) {
if (err == 0)
--
2.1.4
^ permalink raw reply related
* [PATCH net-next] net: ipv4: Remove flow arg from ip_mkroute_input
From: David Ahern @ 2017-01-07 1:39 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
fl4 arg is not used; remove it.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
net/ipv4/route.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 7b52ac20145b..b9bff78274bf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1758,7 +1758,6 @@ static int ip_multipath_icmp_hash(struct sk_buff *skb)
static int ip_mkroute_input(struct sk_buff *skb,
struct fib_result *res,
- const struct flowi4 *fl4,
struct in_device *in_dev,
__be32 daddr, __be32 saddr, u32 tos)
{
@@ -1883,7 +1882,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res.type != RTN_UNICAST)
goto martian_destination;
- err = ip_mkroute_input(skb, &res, &fl4, in_dev, daddr, saddr, tos);
+ err = ip_mkroute_input(skb, &res, in_dev, daddr, saddr, tos);
out: return err;
brd_input:
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next V2 0/3] net/sched: act_pedit: Use offset relative to conventional network headers
From: David Miller @ 2017-01-07 1:51 UTC (permalink / raw)
To: amir; +Cc: netdev, jiri, ogerlitz, hadarh
In-Reply-To: <20170105095454.32644-1-amir@vadai.me>
From: Amir Vadai <amir@vadai.me>
Date: Thu, 5 Jan 2017 11:54:51 +0200
> Enhancing the UAPI to allow for specifying that would allow the same
> flows to be set into both SW and HW.
This is actually not backward compatible.
When pedit rules are dumped, older tools will not know about the
type field and therefore will completely misinterpret the rule.
You must extend this the proper way, which is to add a new attribute
or something along those lines. The presense of a new attribute
is an explicit communication to older tools that somethng they
might not support and understand is going on.
^ permalink raw reply
* Re: [PATCH net] udp: inuse checks can quit early for reuseport
From: David Miller @ 2017-01-07 1:57 UTC (permalink / raw)
To: e; +Cc: netdev
In-Reply-To: <20170106012236.20078-1-e@erig.me>
From: Eric Garver <e@erig.me>
Date: Thu, 5 Jan 2017 20:22:36 -0500
> UDP lib inuse checks will walk the entire hash bucket to check if the
> portaddr is in use. In the case of reuseport we can stop searching when
> we find a matching reuseport.
>
> On a 16-core VM a test program that spawns 16 threads that each bind to
> 1024 sockets (one per 10ms) takes 1m45s. With this change it takes 11s.
>
> Also add a cond_resched() when the port is not specified.
>
> Signed-off-by: Eric Garver <e@erig.me>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v5 0/3] adding new glue driver dwmac-dwc-qos-eth
From: David Miller @ 2017-01-07 2:03 UTC (permalink / raw)
To: Joao.Pinto
Cc: lars.persson, niklass, swarren, treding, netdev, nathan.sullivan
In-Reply-To: <cover.1483697766.git.jpinto@synopsys.com>
From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Fri, 6 Jan 2017 10:48:30 +0000
> This patch set contains the porting of the synopsys/dwc_eth_qos.c driver
> to the stmmac structure. This operation resulted in the creation of a new
> platform glue driver called dwmac-dwc-qos-eth which was based in the
> dwc_eth_qos as is.
>
> dwmac-dwc-qos-eth inherited dwc_eth_qos DT bindings, to assure that current
> and old users can continue to use it as before. We can see this driver as
> being deprecated, since all new development will be done in stmmac.
>
> Please check each patch for implementation details.
Patch #3 doesn't apply cleanly, respin your patch against the net-next
tree please.
^ permalink raw reply
* Re: [PATCHv3 net-next] sctp: prepare asoc stream for stream reconf
From: David Miller @ 2017-01-07 2:07 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich
In-Reply-To: <efd6462731ca0b18a3039f9537dda61e0ed72430.1483712313.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 6 Jan 2017 22:18:33 +0800
> sctp stream reconf, described in RFC 6525, needs a structure to
> save per stream information in assoc, like stream state.
>
> In the future, sctp stream scheduler also needs it to save some
> stream scheduler params and queues.
>
> This patchset is to prepare the stream array in assoc for stream
> reconf. It defines sctp_stream that includes stream arrays inside
> to replace ssnmap.
>
> Note that we use different structures for IN and OUT streams, as
> the members in per OUT stream will get more and more different
> from per IN stream.
>
> v1->v2:
> - put these patches into a smaller group.
> v2->v3:
> - define sctp_stream to contain stream arrays, and create stream.c
> to put stream-related functions.
> - merge 3 patches into 1, as new sctp_stream has the same name
> with before.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH next v1] ipvlan: don't use IDR for generating dev_id
From: David Miller @ 2017-01-07 2:18 UTC (permalink / raw)
To: mahesh; +Cc: netdev, maheshb, edumazet
In-Reply-To: <20170107003311.22940-1-mahesh@bandewar.net>
From: Mahesh Bandewar <mahesh@bandewar.net>
Date: Fri, 6 Jan 2017 16:33:11 -0800
> From: Mahesh Bandewar <maheshb@google.com>
>
> The patch 009146d117b ("ipvlan: assign unique dev-id for each slave
> device.") used ida_simple_get() to generate dev_ids assigned to the
> slave devices. However (Eric has pointed out that) there is a shortcoming
> with that approach as it always uses the first available ID. This
> becomes a problem when a slave gets deleted and a new slave gets added.
> The ID gets reassigned causing the new slave to get the same link-local
> address. This side-effect is undesirable.
>
> This patch replaces IDR logic with a simple per-port variable that keeps
> incrementing and wraps around when the MAX (0xFFFE) is reached. The
> only downside is that this is an inefficient (n^2) search if there are
> 64k (or close to 64k) slaves in the system, the dev-id search takes time.
> However having these many devices in the system has it's own challenges.
>
> Fixes: 009146d117b ("ipvlan: assign unique dev-id for each slave device.")
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
I kind of cringe when I see yet another implementation of an integer ID
allocator.
I think it's much simpler to keep using ida_simple_alloc(), but alongside
it have start point you maintain based upon previous allocations. Put it
in the ipvl_port, just like dev_id_base, but call it "dev_id_start".
Then your ID allocation sequence becomes:
err = ida_simple_get(&port->ida, port->dev_id_start, 0xFFFE, GFP_KERNEL);
if (err < 0)
err = ida_simple_get(&port->ida, 0, port->dev_id_start, GFP_KERNEL);
if (err < 0)
goto destroy_ipvlan_port;
dev->dev_id = err;
port->dev_id_start = err;
if (port->dev_id_start = 0FFFE)
port->dev_id_start = 0;
Something like that.
Alternatively, IDR/IDA can be extended to have this kind of functionality
too.
^ permalink raw reply
* Re: [PATCH net-next 0/2] afs: Implement bulk read
From: David Miller @ 2017-01-07 2:52 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <148372251997.8578.5117118854142528477.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Fri, 06 Jan 2017 17:08:40 +0000
> This pair of patches implements bulk data reading from an AFS server.
>
> The patches can be found here also:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite
>
> Tagged thusly:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
> rxrpc-rewrite-20170106
Pulled, thanks David.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox