* Re: [PATCH net-next 0/2] Add mqprio hardware offload support in hns3 driver
From: Yunsheng Lin @ 2017-10-16 1:55 UTC (permalink / raw)
To: Yuval Mintz, davem@davemloft.net
Cc: huangdaode@hisilicon.com, xuwei5@hisilicon.com,
liguozhu@hisilicon.com, Yisen.Zhuang@huawei.com,
gabriele.paoloni@huawei.com, john.garry@huawei.com,
linuxarm@huawei.com, salil.mehta@huawei.com, lipeng321@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
amritha.nambiar@intel.com
In-Reply-To: <AM0PR0502MB368382B73695B8DA7B360E71BF4E0@AM0PR0502MB3683.eurprd05.prod.outlook.com>
Hi, Yuval
On 2017/10/15 16:51, Yuval Mintz wrote:
>>>>> This patchset adds a new hardware offload type in mqprio before
>> adding
>>>>> mqprio hardware offload support in hns3 driver.
>
> Apparently Dave has already accepted Amirtha's changes to mqprio:
> https://marc.info/?l=linux-netdev&m=150803219824053&w=2
> so I guess you need to revise your patchs to align to the new conventions.
Ok.
"If offloads are supported by setting the 'hw' option to 1, the default
offload mode is 'dcb' where only the TC values are offloaded to the
device. "
According to the description of the above patchset, the default mode is already
dcb, so i will drop the dcb mode patch.
I think the scenario you mentioned still existed, and I am willing to implement
it if we come to a solution that will suit most in the community.
Thanks,
Yunsheng Lin
>
>>>>
>>>> I think one of the biggest issues in tying this to DCB configuration is the
>>>> non-immediate [and possibly non persistent] configuration.
>>>>
>>>> Scenario #1:
>>>> User is configuring mqprio offloaded with 3 TCs while device is in willing
>>> mode.
>>>> Would you expect the driver to immediately respond with a success or
>>> instead
>>>> delay the return until the DCBx negotiation is complete and the
>> operational
>>>> num of TCs is actually 3?
>>>
>>> Well, when user requsts the mqprio offloaded by a hardware shared by
>> DCB,
>>> I expect
>>> the user is not using the dcb tool.
>>> If user is still using dcb tool, then result is undefined.
>>>
>>> The scenario you mention maybe can be enforced by setting willing to zero
>>> when user
>>> is requesting the mqprio offload, and restore the willing bit when unloaded
>>> the mqprio
>>> offload.
>>
>> Sounds a bit harsh but would probably work.
>>
>>> But I think the real issue is that dcb and mqprio shares the tc system in the
>>> stack,
>>> the problem may be better to be fixed in the stack rather than in the
>> driver,
>>> as you
>>> suggested in the DCB patchset. What do you think?
>>
>> What did you have in mind?
>>
>>>
>>>>
>>>> Scenario #2:
>>>> Assume user explicitly offloaded mqprio with 3 TCs, but now DCB
>>> configuration
>>>> has changed on the peer side and 4 TCs is the new negotiated operational
>>> value.
>>>> Your current driver logic would change the number of TCs underneath
>> the
>>> user
>>>> configuration [and it would actually probably work due to mqprio being a
>>> crappy
>>>> qdisc]. But was that the user actual intention?
>>>> [I think the likely answer in this scenario is 'yes' since the alternative is no
>>> better.
>>>> But I still thought it was worth mentioning]
>>>
>>> You are right, the problem also have something to do with mqprio and dcb
>>> sharing
>>> the tc in the stack.
>>>
>>> Druing testing, when user explicitly offloaded mqprio with 3 TCs, all
>>> queue has a default pfifo mqprio attached, after DCB changes the tc num
>> to
>>> 4,
>>> using tc qdisc shows some queue does not have a default pfifo mqprio
>>> attached.
>>
>> Really? Then what did it show?
>> [I assume it has some pfifo attached, and it's an mqprio dump kind of an
>> issue]
>>
>>>
>>> Maybe we can add a callback to notify mqprio the configuration has
>> changed.
>>>
>>
>> Which would do what?
>> You already have the notifications available for monitoring using dcbnl logic if
>> the
>> configuration change [for user]; So user can re-configure whatever it wants.
>> But other than dropping all the qdisc configurations and going back to the
>> default
>> qdiscs, what default action would mqprio be able to do when configuration
>> changes
>> that actually makes sense?
>>
>>> Thanks
>>> Yunsheng Lin
>>>
>>>>
>>>> Cheers,
>>>> Yuval
>>>>
>>>>>
>>>>> Yunsheng Lin (2):
>>>>> mqprio: Add a new hardware offload type in mqprio
>>>>> net: hns3: Add mqprio hardware offload support in hns3 driver
>>>>>
>>>>> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 +
>>>>> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 23 +++++++++++
>>>>> .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46
>>> ++++++++++++++-
>>>>> -------
>>>>> include/uapi/linux/pkt_sched.h | 1 +
>>>>> 4 files changed, 55 insertions(+), 16 deletions(-)
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>
>>>>
>>>>
>
^ permalink raw reply
* Re: routing UAPI mismatch invalid state behavior?
From: David Ahern @ 2017-10-16 1:56 UTC (permalink / raw)
To: Alexander Aring, netdev; +Cc: Jamal Hadi Salim
In-Reply-To: <CAOHTApje2ABpRPghhqabD=JkBOzjFjVmc1N67ReDBLHeGGVe1Q@mail.gmail.com>
On 10/15/17 11:02 AM, Alexander Aring wrote:
> Hi,
>
> I figure out some problem, easy to reproduce:
>
> # setup dummy
> $ modprobe dummy
> $ ip link set dummy0 up
>
> # issue
> $ ip route replace default via 169.254.65.37 dev dummy0
> RTNETLINK answers: Network is unreachable
This fails in fib_check_nh. Basically the lookup of the address fails
making it an invalid gateway.
>
> so it will forbid me to do that, but:
>
> $ ip route replace 169.254.65.37 dev dummy0
This succeeds because iproute2 adds NLM_F_CREATE to flags along with
REPLACE:
if (matches(*argv, "replace") == 0)
return iproute_modify(RTM_NEWROUTE,
NLM_F_CREATE|NLM_F_REPLACE,
argc-1, argv+1);
but your overall intent here of resolving 169.254.65.37 is the key.
> $ ip route replace default via 169.254.65.37 dev dummy0
The existence of the previous route allows the gateway check in
fib_check_nh to succeed.
> $ ip route del 169.254.65.37 dev dummy0
>
> allows me to do that. Is there now a invalid state in my routing table
> or is it an expected behavior?
There are no recursive checks to gateway lookups once the routes are
installed. With the way routes and nexthops are currently created doing
so would be very expensive in some setups.
^ permalink raw reply
* To learn the netdev code, please give me some advice...
From: admin @ 2017-10-16 2:23 UTC (permalink / raw)
To: netdev
Hi, all
I want to learn network programming, and hope to read the kernel
code of netdev.
Please give me some advice.
Thanks!
^ permalink raw reply
* RE: [PATCH v2 5/5] fsl/fman: add dpaa in module names
From: Madalin-cristian Bucur @ 2017-10-16 3:15 UTC (permalink / raw)
To: Florian Fainelli, netdev@vger.kernel.org, davem@davemloft.net
Cc: andrew@lunn.ch, vivien.didelot@savoirfairelinux.com,
junote@outlook.com, linux-kernel@vger.kernel.org
In-Reply-To: <ba387138-03e5-52df-dc5b-98c6c9d57be8@gmail.com>
> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Friday, October 13, 2017 8:39 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>;
> netdev@vger.kernel.org; davem@davemloft.net
> Cc: andrew@lunn.ch; vivien.didelot@savoirfairelinux.com;
> junote@outlook.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 5/5] fsl/fman: add dpaa in module names
>
> On 10/13/2017 07:50 AM, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
>
> You should provide a line or two to explain why are you making this
> change, it is to resolve modular build configurations?
This change just renames the FMan driver modules, using a common prefix for
the DPAA FMan and DPAA Ethernet drivers. Besides making the names more aligned,
this allows writing udev rules that match on either driver name, if needed,
using the fsl_dpaa_* prefix. The change of netdev dev required for the DSA
probing makes the previous rules written using this prefix fail, this change
makes them work again.
> > ---
> > drivers/net/ethernet/freescale/fman/Makefile | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fman/Makefile
> b/drivers/net/ethernet/freescale/fman/Makefile
> > index 2c38119..4ae524a 100644
> > --- a/drivers/net/ethernet/freescale/fman/Makefile
> > +++ b/drivers/net/ethernet/freescale/fman/Makefile
> > @@ -1,9 +1,9 @@
> > subdir-ccflags-y += -I$(srctree)/drivers/net/ethernet/freescale/fman
> >
> > -obj-$(CONFIG_FSL_FMAN) += fsl_fman.o
> > -obj-$(CONFIG_FSL_FMAN) += fsl_fman_port.o
> > -obj-$(CONFIG_FSL_FMAN) += fsl_mac.o
> > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman.o
> > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_fman_port.o
> > +obj-$(CONFIG_FSL_FMAN) += fsl_dpaa_mac.o
> >
> > -fsl_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o
> > -fsl_fman_port-objs := fman_port.o
> > -fsl_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
> > +fsl_dpaa_fman-objs := fman_muram.o fman.o fman_sp.o fman_keygen.o
> > +fsl_dpaa_fman_port-objs := fman_port.o
> > +fsl_dpaa_mac-objs:= mac.o fman_dtsec.o fman_memac.o fman_tgec.o
> >
>
>
> --
> Florian
^ permalink raw reply
* RE: [PATCH net v2 2/2] net: fec: Let fec_ptp have its own interrupt routine
From: Andy Duan @ 2017-10-16 3:41 UTC (permalink / raw)
To: Troy Kisky, shawn.guo@linaro.org, netdev@vger.kernel.org,
davem@davemloft.net
Cc: Fabio Estevam, lznuaa@gmail.com, andrew@lunn.ch
In-Reply-To: <20171014020940.32736-2-troy.kisky@boundarydevices.com>
From: Troy Kisky <troy.kisky@boundarydevices.com> Sent: Saturday, October 14, 2017 10:10 AM
>This is better for code locality and should slightly speed up normal interrupts.
>
>This also allows PPS clock output to start working for i.mx7. This is because
>i.mx7 was already using the limit of 3 interrupts, and needed another.
>
>Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>
>---
>
>v2: made this change independent of any devicetree change so that old dtbs
>continue to work.
>
>Continue to register ptp clock if interrupt is not found.
>---
> drivers/net/ethernet/freescale/fec.h | 3 +-
> drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
>drivers/net/ethernet/freescale/fec_ptp.c | 82 ++++++++++++++++++--------
>-----
> 3 files changed, 65 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index ede1876a9a19..be56ac1f1ac4 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -582,12 +582,11 @@ struct fec_enet_private {
> u64 ethtool_stats[0];
> };
>
>-void fec_ptp_init(struct platform_device *pdev);
>+void fec_ptp_init(struct platform_device *pdev, int irq_index);
> void fec_ptp_stop(struct platform_device *pdev); void
>fec_ptp_start_cyclecounter(struct net_device *ndev); int fec_ptp_set(struct
>net_device *ndev, struct ifreq *ifr); int fec_ptp_get(struct net_device *ndev,
>struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private
>*fep);
>
>
>/**********************************************************
>******************/
> #endif /* FEC_H */
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 3dc2d771a222..21afabbc560f 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id)
> ret = IRQ_HANDLED;
> complete(&fep->mdio_done);
> }
>-
>- if (fep->ptp_clock)
>- if (fec_ptp_check_pps_event(fep))
>- ret = IRQ_HANDLED;
> return ret;
> }
>
>@@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node, *phy_node;
> int num_tx_qs;
> int num_rx_qs;
>+ char irq_name[8];
>+ int irq_cnt;
>
> fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
>
>@@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
> if (ret)
> goto failed_reset;
>
>+ irq_cnt = platform_irq_count(pdev);
>+ if (irq_cnt > FEC_IRQ_NUM)
>+ irq_cnt = FEC_IRQ_NUM; /* last for ptp */
>+ else if (irq_cnt == 2)
>+ irq_cnt = 1; /* last for ptp */
>+ else if (irq_cnt <= 0)
>+ irq_cnt = 1; /* Let the for loop fail */
Don't do like this. Don't suppose pps interrupt is the last one.
And if irq_cnt is 1 like imx28/imx5x, the patch will break fec interrupt function.
I suggest to use .platform_get_irq_byname() to get pps(ptp) interrupt like your v1 logic check.
>+
> if (fep->bufdesc_ex)
>- fec_ptp_init(pdev);
>+ fec_ptp_init(pdev, irq_cnt);
>
> ret = fec_enet_init(ndev);
> if (ret)
> goto failed_init;
>
>- for (i = 0; i < FEC_IRQ_NUM; i++) {
>- irq = platform_get_irq(pdev, i);
>+ for (i = 0; i < irq_cnt; i++) {
>+ sprintf(irq_name, "int%d", i);
>+ irq = platform_get_irq_byname(pdev, irq_name);
>+ if (irq < 0)
>+ irq = platform_get_irq(pdev, i);
> if (irq < 0) {
>- if (i)
>- break;
> ret = irq;
> goto failed_irq;
> }
>diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>b/drivers/net/ethernet/freescale/fec_ptp.c
>index 6ebad3fac81d..3abeee0d16dd 100644
>--- a/drivers/net/ethernet/freescale/fec_ptp.c
>+++ b/drivers/net/ethernet/freescale/fec_ptp.c
>@@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work)
> schedule_delayed_work(&fep->time_keep, HZ); }
>
>+/* This function checks the pps event and reloads the timer compare
>+counter. */ static irqreturn_t fec_ptp_interrupt(int irq, void *dev_id)
>+{
>+ struct net_device *ndev = dev_id;
>+ struct fec_enet_private *fep = netdev_priv(ndev);
>+ u32 val;
>+ u8 channel = fep->pps_channel;
>+ struct ptp_clock_event event;
>+
>+ val = readl(fep->hwp + FEC_TCSR(channel));
>+ if (val & FEC_T_TF_MASK) {
>+ /* Write the next next compare(not the next according the
>spec)
>+ * value to the register
>+ */
>+ writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>+ do {
>+ writel(val, fep->hwp + FEC_TCSR(channel));
>+ } while (readl(fep->hwp + FEC_TCSR(channel)) &
>FEC_T_TF_MASK);
>+
>+ /* Update the counter; */
>+ fep->next_counter = (fep->next_counter + fep-
>>reload_period) &
>+ fep->cc.mask;
>+
>+ event.type = PTP_CLOCK_PPS;
>+ ptp_clock_event(fep->ptp_clock, &event);
>+ return IRQ_HANDLED;
>+ }
>+
>+ return IRQ_NONE;
>+}
>+
> /**
> * fec_ptp_init
> * @ndev: The FEC network adapter
>@@ -558,10 +589,12 @@ static void fec_time_keep(struct work_struct *work)
> * cyclecounter init routine and exits.
> */
>
>-void fec_ptp_init(struct platform_device *pdev)
>+void fec_ptp_init(struct platform_device *pdev, int irq_index)
> {
> struct net_device *ndev = platform_get_drvdata(pdev);
> struct fec_enet_private *fep = netdev_priv(ndev);
>+ int irq;
>+ int ret;
>
> fep->ptp_caps.owner = THIS_MODULE;
> snprintf(fep->ptp_caps.name, 16, "fec ptp"); @@ -587,6 +620,20 @@
>void fec_ptp_init(struct platform_device *pdev)
>
> INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);
>
>+ irq = platform_get_irq_byname(pdev, "ptp");
>+ if (irq < 0)
>+ irq = platform_get_irq(pdev, irq_index);
>+ /* Failure to get an irq is not fatal,
>+ * only the PTP_CLOCK_PPS clock events should stop
>+ */
>+ if (irq >= 0) {
>+ ret = devm_request_irq(&pdev->dev, irq, fec_ptp_interrupt,
>+ 0, pdev->name, ndev);
>+ if (ret < 0)
>+ dev_warn(&pdev->dev, "request for ptp irq
>failed(%d)\n",
>+ ret);
>+ }
>+
> fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
> if (IS_ERR(fep->ptp_clock)) {
> fep->ptp_clock = NULL;
>@@ -605,36 +652,3 @@ void fec_ptp_stop(struct platform_device *pdev)
> if (fep->ptp_clock)
> ptp_clock_unregister(fep->ptp_clock);
> }
>-
>-/**
>- * fec_ptp_check_pps_event
>- * @fep: the fec_enet_private structure handle
>- *
>- * This function check the pps event and reload the timer compare counter.
>- */
>-uint fec_ptp_check_pps_event(struct fec_enet_private *fep) -{
>- u32 val;
>- u8 channel = fep->pps_channel;
>- struct ptp_clock_event event;
>-
>- val = readl(fep->hwp + FEC_TCSR(channel));
>- if (val & FEC_T_TF_MASK) {
>- /* Write the next next compare(not the next according the
>spec)
>- * value to the register
>- */
>- writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>- do {
>- writel(val, fep->hwp + FEC_TCSR(channel));
>- } while (readl(fep->hwp + FEC_TCSR(channel)) &
>FEC_T_TF_MASK);
>-
>- /* Update the counter; */
>- fep->next_counter = (fep->next_counter + fep-
>>reload_period) & fep->cc.mask;
>-
>- event.type = PTP_CLOCK_PPS;
>- ptp_clock_event(fep->ptp_clock, &event);
>- return 1;
>- }
>-
>- return 0;
>-}
>--
>2.11.0
^ permalink raw reply
* Re: [PATCH 3/3][v2] selftests: silence test output by default
From: Michael Ellerman @ 2017-10-16 3:57 UTC (permalink / raw)
To: Shuah Khan, josef, davem, netdev, linux-kselftest; +Cc: Josef Bacik, Shuah Khan
In-Reply-To: <89b70a11-27d9-9b56-2010-66258618cdb4@kernel.org>
Shuah Khan <shuah@kernel.org> writes:
> On 09/19/2017 07:51 AM, josef@toxicpanda.com wrote:
>> From: Josef Bacik <jbacik@fb.com>
>>
>> Some of the networking tests are very noisy and make it impossible to
>> see if we actually passed the tests as they run. Default to suppressing
>> the output from any tests run in order to make it easier to track what
>> failed.
>>
>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>> ---
>> v1->v2:
>> - dump output into /tmp/testname instead of /dev/null
>>
>
> Thanks for the fix. Applied to linux-kselftest for 4.14-rc2
Sorry this is not a fix.
This is a regression, it breaks all my test infrastructure, because we
use the output of the test case.
Can we please revert this and fix any tests that are overly verbose.
cheers
^ permalink raw reply
* RE: [PATCH v2 2/5] dpaa_eth: move of_phy_connect() to the eth driver
From: Madalin-cristian Bucur @ 2017-10-16 4:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, davem@davemloft.net, f.fainelli@gmail.com,
vivien.didelot@savoirfairelinux.com, junote@outlook.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20171015183349.GD6374@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Sunday, October 15, 2017 9:34 PM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; f.fainelli@gmail.com;
> vivien.didelot@savoirfairelinux.com; junote@outlook.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 2/5] dpaa_eth: move of_phy_connect() to the eth
> driver
>
> On Fri, Oct 13, 2017 at 05:50:09PM +0300, Madalin Bucur wrote:
> > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> > ---
> > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 48 +++++++++++--
> > drivers/net/ethernet/freescale/fman/mac.c | 97 ++++++-------------
> -------
> > drivers/net/ethernet/freescale/fman/mac.h | 5 +-
> > 3 files changed, 66 insertions(+), 84 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > index 4225806..7cf61d6 100644
> > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> > @@ -2435,6 +2435,48 @@ static void dpaa_eth_napi_disable(struct
> dpaa_priv *priv)
> > }
> > }
> >
> > +static void dpaa_adjust_link(struct net_device *net_dev)
> > +{
> > + struct mac_device *mac_dev;
> > + struct dpaa_priv *priv;
> > +
> > + priv = netdev_priv(net_dev);
> > + mac_dev = priv->mac_dev;
> > + mac_dev->adjust_link(mac_dev);
> > +}
> > +
> > +static int dpaa_phy_init(struct net_device *net_dev)
> > +{
> > + struct mac_device *mac_dev;
> > + struct phy_device *phy_dev;
> > + struct dpaa_priv *priv;
> > +
> > + priv = netdev_priv(net_dev);
> > + mac_dev = priv->mac_dev;
> > +
> > + phy_dev = of_phy_connect(net_dev, mac_dev->phy_node,
> > + &dpaa_adjust_link, 0,
> > + mac_dev->phy_if);
> > + if (!phy_dev) {
> > + netif_err(priv, ifup, net_dev, "init_phy() failed\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Remove any features not supported by the controller */
> > + phy_dev->supported &= mac_dev->if_support;
> > +
> > + /* Enable the symmetric and asymmetric PAUSE frame advertisements,
> > + * as most of the PHY drivers do not enable them by default.
> > + */
>
> Hi Madalin
>
> This is just moving code around, so the patch is O.K. However, it
> would be nice to have a followup patch. This comment is wrong. The phy
> driver should never enable symmetric and asymmetric PAUSE frames. The
> MAC needs to, because only the MAC knows if the MAC supports pause
> frames.
>
> Andrew
Hi Andrew,
This is obsolete and it will be removed, I'll send a v3. It remained there from
a time it used to be valid (the original DPAA Ethernet driver was developed
and maintained out of tree since about 9 years ago). I see this thread on the
subject which is relatively recent:
https://www.spinics.net/lists/netdev/msg404288.html
Thanks,
Madalin
^ permalink raw reply
* Re: [pull request][for-next 00/12] Mellanox, mlx5 IPoIB Muli Pkey support 2017-10-11,Re: [pull request][for-next 00/12] Mellanox, mlx5 IPoIB Muli Pkey support 2017-10-11
From: David Miller @ 2017-10-16 4:45 UTC (permalink / raw)
To: dledford; +Cc: saeedm, netdev, linux-rdma, leonro
In-Reply-To: <af28440c-bb1c-c47b-92dd-60bfbeaa839d@redhat.com>
From: Doug Ledford <dledford@redhat.com>
Date: Sat, 14 Oct 2017 17:19:34 -0400
> On 10/14/2017 2:48 PM, Saeed Mahameed wrote:
>> Hi Dave and Doug,
>>
>> This series includes updates for mlx5 IPoIB offloading driver from Alex
>> and Feras to add the support for Muli Pkey in the mlx5i ipoib offloading netdev,
>> to be merged into net-next and rdma-next trees.
>
> As far as the two IPoIB patches are concerned, they're fine.
>
>> Doug, I am sorry I couldn't base this on rc2 since the series needs and conflicts
>> with a fix that was submitted to rc3, so to keep things simple I based it on rc4,
>> I hope this is ok with you..
>
> No worries, it just means I have to submit it under another branch. But
> I'm already holding one patch series in a stand alone branch, so no big
> deal. And, actually, the IPoIB changes are so small they can simply go
> through Dave's tree if you don't have any dependent code in the IPoIB
> driver to submit after this but still in this devel cycle.
>
>> Please pull and let me know if there's any problem.
>
> Once I hear that Dave is OK with the net changes, I'm ready to pull (if
> I need to).
They look fine and I've pulled this into net-next.
Thanks.
^ permalink raw reply
* Re: [PATCH v1] pch_gbe: Switch to new PCI IRQ allocation API
From: kbuild test robot @ 2017-10-16 6:23 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: kbuild-all, David S. Miller, netdev, Andy Shevchenko
In-Reply-To: <20171013170221.70056-1-andriy.shevchenko@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2304 bytes --]
Hi Andy,
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.14-rc5 next-20171013]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/pch_gbe-Switch-to-new-PCI-IRQ-allocation-API/20171016-131405
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c: In function 'pch_gbe_request_irq':
>> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1904:30: error: 'pdev' undeclared (first use in this function)
err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
^
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c:1904:30: note: each undeclared identifier is reported only once for each function it appears in
vim +/pdev +1904 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
1891
1892 /**
1893 * pch_gbe_request_irq - Allocate an interrupt line
1894 * @adapter: Board private structure
1895 * Returns:
1896 * 0: Successfully
1897 * Negative value: Failed
1898 */
1899 static int pch_gbe_request_irq(struct pch_gbe_adapter *adapter)
1900 {
1901 struct net_device *netdev = adapter->netdev;
1902 int err;
1903
> 1904 err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
1905 if (err < 0)
1906 return err;
1907
1908 adapter->irq = pci_irq_vector(pdev, 0);
1909
1910 err = request_irq(adapter->irq, &pch_gbe_intr, IRQF_SHARED,
1911 netdev->name, netdev);
1912 if (err)
1913 netdev_err(netdev, "Unable to allocate interrupt Error: %d\n",
1914 err);
1915 netdev_dbg(netdev, "have_msi : %d return : 0x%04x\n",
1916 pci_dev_msi_enabled(adapter->pdev), err);
1917 return err;
1918 }
1919
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51658 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 0/2] Add mqprio hardware offload support in hns3 driver
From: Yuval Mintz @ 2017-10-16 6:25 UTC (permalink / raw)
To: Yunsheng Lin, davem@davemloft.net
Cc: huangdaode@hisilicon.com, xuwei5@hisilicon.com,
liguozhu@hisilicon.com, Yisen.Zhuang@huawei.com,
gabriele.paoloni@huawei.com, john.garry@huawei.com,
linuxarm@huawei.com, salil.mehta@huawei.com, lipeng321@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <f651e2c2-5233-bcc7-7488-3ec5905b92d4@huawei.com>
> Hi, Yuval
>
> On 2017/10/15 13:14, Yuval Mintz wrote:
> >> Hi, Yuval
> >>
> >> On 2017/10/13 4:21, Yuval Mintz wrote:
> >>>> This patchset adds a new hardware offload type in mqprio before
> adding
> >>>> mqprio hardware offload support in hns3 driver.
> >>>
> >>> I think one of the biggest issues in tying this to DCB configuration is the
> >>> non-immediate [and possibly non persistent] configuration.
> >>>
> >>> Scenario #1:
> >>> User is configuring mqprio offloaded with 3 TCs while device is in willing
> >> mode.
> >>> Would you expect the driver to immediately respond with a success or
> >> instead
> >>> delay the return until the DCBx negotiation is complete and the
> operational
> >>> num of TCs is actually 3?
> >>
> >> Well, when user requsts the mqprio offloaded by a hardware shared by
> DCB,
> >> I expect
> >> the user is not using the dcb tool.
> >> If user is still using dcb tool, then result is undefined.
> >>
> >> The scenario you mention maybe can be enforced by setting willing to
> zero
> >> when user
> >> is requesting the mqprio offload, and restore the willing bit when
> unloaded
> >> the mqprio
> >> offload.
> >
> > Sounds a bit harsh but would probably work.
> >
> >> But I think the real issue is that dcb and mqprio shares the tc system in the
> >> stack,
> >> the problem may be better to be fixed in the stack rather than in the
> driver,
> >> as you
> >> suggested in the DCB patchset. What do you think?
> >
> > What did you have in mind?
>
> I was thinking maybe the tc system can provide a notification to mqprio and
> dcb.
> mqprio and dcb register a callback to the tc system, when there is some
> change of
> tc configuration, the tc system call the callback from mqprio and dcb.
>
> >
> >>
> >>>
> >>> Scenario #2:
> >>> Assume user explicitly offloaded mqprio with 3 TCs, but now DCB
> >> configuration
> >>> has changed on the peer side and 4 TCs is the new negotiated
> operational
> >> value.
> >>> Your current driver logic would change the number of TCs underneath
> the
> >> user
> >>> configuration [and it would actually probably work due to mqprio being a
> >> crappy
> >>> qdisc]. But was that the user actual intention?
> >>> [I think the likely answer in this scenario is 'yes' since the alternative is no
> >> better.
> >>> But I still thought it was worth mentioning]
> >>
> >> You are right, the problem also have something to do with mqprio and dcb
> >> sharing
> >> the tc in the stack.
> >>
> >> Druing testing, when user explicitly offloaded mqprio with 3 TCs, all
> >> queue has a default pfifo mqprio attached, after DCB changes the tc num
> to
> >> 4,
> >> using tc qdisc shows some queue does not have a default pfifo mqprio
> >> attached.
> >
> > Really? Then what did it show?
> > [I assume it has some pfifo attached, and it's an mqprio dump kind of an
> issue]
>
> When queue size of the ndev is 16 and tc num is 3, we set the real queue size
> to
> 15 ( 5 * 3 = 15), mqprio only attach pfifo to the first 15 queue, when tc num
> change
> to 4 by DCB, we set the real queue size to 16 (4 * 4 = 16).
> So tc qdisc shows the last queue has no qdisc attached.
So there is a qdisc attached - mqprio_attach() attches to all transmission
queues [num_tx_queues] and not only the active ones.
But the flow for mqprio might be lacking the additional qdisc_hash_add()
for the additional queue's qdisc.
>
> >
> >>
> >> Maybe we can add a callback to notify mqprio the configuration has
> changed.
> >>
> >
> > Which would do what?
> > You already have the notifications available for monitoring using dcbnl logic
> if the
> > configuration change [for user]; So user can re-configure whatever it
> wants.
>
> Yes, if user is only using dcb tool.
>
> > But other than dropping all the qdisc configurations and going back to the
> default
> > qdiscs, what default action would mqprio be able to do when configuration
> changes
> > that actually makes sense?
>
> As explained above, after dcb changing the configuration, some queue may
> have no qdisc
> attached, so I was thinking maybe we can add pfifo to it if there is no qdsic
> attached
> to it.
>
> Thanks,
> Yunsheng Lin
>
> >
> >> Thanks
> >> Yunsheng Lin
> >>
> >>>
> >>> Cheers,
> >>> Yuval
> >>>
> >>>>
> >>>> Yunsheng Lin (2):
> >>>> mqprio: Add a new hardware offload type in mqprio
> >>>> net: hns3: Add mqprio hardware offload support in hns3 driver
> >>>>
> >>>> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 +
> >>>> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 23
> +++++++++++
> >>>> .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46
> >> ++++++++++++++-
> >>>> -------
> >>>> include/uapi/linux/pkt_sched.h | 1 +
> >>>> 4 files changed, 55 insertions(+), 16 deletions(-)
> >>>>
> >>>> --
> >>>> 1.9.1
> >>>
> >>>
> >>>
> >
^ permalink raw reply
* Re: [PATCH net-next 0/2] Add mqprio hardware offload support in hns3 driver
From: Yunsheng Lin @ 2017-10-16 7:05 UTC (permalink / raw)
To: Yuval Mintz, davem@davemloft.net
Cc: huangdaode@hisilicon.com, xuwei5@hisilicon.com,
liguozhu@hisilicon.com, Yisen.Zhuang@huawei.com,
gabriele.paoloni@huawei.com, john.garry@huawei.com,
linuxarm@huawei.com, salil.mehta@huawei.com, lipeng321@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <AM0PR0502MB3683372D5A1C9DF65431113CBF4F0@AM0PR0502MB3683.eurprd05.prod.outlook.com>
Hi, Yuval
On 2017/10/16 14:25, Yuval Mintz wrote:
>> Hi, Yuval
>>
>> On 2017/10/15 13:14, Yuval Mintz wrote:
>>>> Hi, Yuval
>>>>
>>>> On 2017/10/13 4:21, Yuval Mintz wrote:
>>>>>> This patchset adds a new hardware offload type in mqprio before
>> adding
>>>>>> mqprio hardware offload support in hns3 driver.
>>>>>
>>>>> I think one of the biggest issues in tying this to DCB configuration is the
>>>>> non-immediate [and possibly non persistent] configuration.
>>>>>
>>>>> Scenario #1:
>>>>> User is configuring mqprio offloaded with 3 TCs while device is in willing
>>>> mode.
>>>>> Would you expect the driver to immediately respond with a success or
>>>> instead
>>>>> delay the return until the DCBx negotiation is complete and the
>> operational
>>>>> num of TCs is actually 3?
>>>>
>>>> Well, when user requsts the mqprio offloaded by a hardware shared by
>> DCB,
>>>> I expect
>>>> the user is not using the dcb tool.
>>>> If user is still using dcb tool, then result is undefined.
>>>>
>>>> The scenario you mention maybe can be enforced by setting willing to
>> zero
>>>> when user
>>>> is requesting the mqprio offload, and restore the willing bit when
>> unloaded
>>>> the mqprio
>>>> offload.
>>>
>>> Sounds a bit harsh but would probably work.
>>>
>>>> But I think the real issue is that dcb and mqprio shares the tc system in the
>>>> stack,
>>>> the problem may be better to be fixed in the stack rather than in the
>> driver,
>>>> as you
>>>> suggested in the DCB patchset. What do you think?
>>>
>>> What did you have in mind?
>>
>> I was thinking maybe the tc system can provide a notification to mqprio and
>> dcb.
>> mqprio and dcb register a callback to the tc system, when there is some
>> change of
>> tc configuration, the tc system call the callback from mqprio and dcb.
>>
>>>
>>>>
>>>>>
>>>>> Scenario #2:
>>>>> Assume user explicitly offloaded mqprio with 3 TCs, but now DCB
>>>> configuration
>>>>> has changed on the peer side and 4 TCs is the new negotiated
>> operational
>>>> value.
>>>>> Your current driver logic would change the number of TCs underneath
>> the
>>>> user
>>>>> configuration [and it would actually probably work due to mqprio being a
>>>> crappy
>>>>> qdisc]. But was that the user actual intention?
>>>>> [I think the likely answer in this scenario is 'yes' since the alternative is no
>>>> better.
>>>>> But I still thought it was worth mentioning]
>>>>
>>>> You are right, the problem also have something to do with mqprio and dcb
>>>> sharing
>>>> the tc in the stack.
>>>>
>>>> Druing testing, when user explicitly offloaded mqprio with 3 TCs, all
>>>> queue has a default pfifo mqprio attached, after DCB changes the tc num
>> to
>>>> 4,
>>>> using tc qdisc shows some queue does not have a default pfifo mqprio
>>>> attached.
>>>
>>> Really? Then what did it show?
>>> [I assume it has some pfifo attached, and it's an mqprio dump kind of an
>> issue]
>>
>> When queue size of the ndev is 16 and tc num is 3, we set the real queue size
>> to
>> 15 ( 5 * 3 = 15), mqprio only attach pfifo to the first 15 queue, when tc num
>> change
>> to 4 by DCB, we set the real queue size to 16 (4 * 4 = 16).
>> So tc qdisc shows the last queue has no qdisc attached.
>
> So there is a qdisc attached - mqprio_attach() attches to all transmission
> queues [num_tx_queues] and not only the active ones.
> But the flow for mqprio might be lacking the additional qdisc_hash_add()
> for the additional queue's qdisc.
Yes, I think you may be right.
static void mqprio_attach(struct Qdisc *sch)
{
struct net_device *dev = qdisc_dev(sch);
struct mqprio_sched *priv = qdisc_priv(sch);
struct Qdisc *qdisc, *old;
unsigned int ntx;
/* Attach underlying qdisc */
for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
qdisc = priv->qdiscs[ntx];
old = dev_graft_qdisc(qdisc->dev_queue, qdisc);
if (old)
qdisc_destroy(old);
----------Only call qdisc_hash_add when ntx < dev->real_num_tx_queues---------------
if (ntx < dev->real_num_tx_queues)
qdisc_hash_add(qdisc, false);
}
kfree(priv->qdiscs);
priv->qdiscs = NULL;
}
>
>>
>>>
>>>>
>>>> Maybe we can add a callback to notify mqprio the configuration has
>> changed.
>>>>
>>>
>>> Which would do what?
>>> You already have the notifications available for monitoring using dcbnl logic
>> if the
>>> configuration change [for user]; So user can re-configure whatever it
>> wants.
>>
>> Yes, if user is only using dcb tool.
>>
>>> But other than dropping all the qdisc configurations and going back to the
>> default
>>> qdiscs, what default action would mqprio be able to do when configuration
>> changes
>>> that actually makes sense?
>>
>> As explained above, after dcb changing the configuration, some queue may
>> have no qdisc
>> attached, so I was thinking maybe we can add pfifo to it if there is no qdsic
>> attached
>> to it.
>>
>> Thanks,
>> Yunsheng Lin
>>
>>>
>>>> Thanks
>>>> Yunsheng Lin
>>>>
>>>>>
>>>>> Cheers,
>>>>> Yuval
>>>>>
>>>>>>
>>>>>> Yunsheng Lin (2):
>>>>>> mqprio: Add a new hardware offload type in mqprio
>>>>>> net: hns3: Add mqprio hardware offload support in hns3 driver
>>>>>>
>>>>>> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 +
>>>>>> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 23
>> +++++++++++
>>>>>> .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46
>>>> ++++++++++++++-
>>>>>> -------
>>>>>> include/uapi/linux/pkt_sched.h | 1 +
>>>>>> 4 files changed, 55 insertions(+), 16 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 1.9.1
>>>>>
>>>>>
>>>>>
>>>
>
^ permalink raw reply
* Re: [net-next RFC 3/4] openvswitch: Add meter infrastructure
From: Andy Zhou @ 2017-10-16 7:05 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <CAOrHB_Cc1cB7mYbYr+uDqsNusP=xALQAWZo=Mr+X9xzgepGFtw@mail.gmail.com>
On Fri, Oct 13, 2017 at 5:12 PM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
>> OVS kernel datapath so far does not support Openflow meter action.
>> This is the first stab at adding kernel datapath meter support.
>> This implementation supports only drop band type.
>>
>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>> ---
>> net/openvswitch/Makefile | 1 +
>> net/openvswitch/datapath.c | 14 +-
>> net/openvswitch/datapath.h | 3 +
>> net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++++++++
>> net/openvswitch/meter.h | 54 ++++
>> 5 files changed, 681 insertions(+), 2 deletions(-)
>> create mode 100644 net/openvswitch/meter.c
>> create mode 100644 net/openvswitch/meter.h
>>
> ...
>
>> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
>> new file mode 100644
>> index 000000000000..f24ebb5f7af4
>> --- /dev/null
>> +++ b/net/openvswitch/meter.c
>
> ....
> ....
>> +static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
>> +{
>> + struct datapath *dp;
>> + struct ovs_header *ovs_header = info->userhdr;
>> + struct sk_buff *reply;
>> + struct ovs_header *ovs_reply_header;
>> + struct nlattr *nla, *band_nla;
>> + int err;
>> +
>> + /* Check that the datapath exists */
>> + ovs_lock();
>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>> + ovs_unlock();
>> + if (!dp)
>> + return -ENODEV;
>> +
> why dp check is required for this API?
Is it possible for another core delete the dp, before ovs_lock()
returns? Then, in theory, get_dp() can
return NULL, no?
>
>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
>> + &ovs_reply_header);
>> + if (!reply)
>> + return PTR_ERR(reply);
>> +
>> + if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
>> + nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
>> + goto nla_put_failure;
>> +
>> + nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
>> + if (!nla)
>> + goto nla_put_failure;
>> +
>> + band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
>> + if (!band_nla)
>> + goto nla_put_failure;
>> + /* Currently only DROP band type is supported. */
>> + if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
>> + goto nla_put_failure;
>> + nla_nest_end(reply, band_nla);
>> + nla_nest_end(reply, nla);
>> +
>> + genlmsg_end(reply, ovs_reply_header);
>> + return genlmsg_reply(reply, info);
>> +
>> +nla_put_failure:
>> + nlmsg_free(reply);
>> + err = -EMSGSIZE;
>> + return err;
>> +}
>> +
> ....
>
>> +static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
>> +{
>> + struct nlattr **a = info->attrs;
>> + struct dp_meter *meter, *old_meter;
>> + struct sk_buff *reply;
>> + struct ovs_header *ovs_reply_header;
>> + struct ovs_header *ovs_header = info->userhdr;
>> + struct datapath *dp;
>> + int err;
>> + u32 meter_id;
>> + bool failed;
>> +
>> + meter = dp_meter_create(a);
>> + if (IS_ERR_OR_NULL(meter))
>> + return PTR_ERR(meter);
>> +
>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
>> + &ovs_reply_header);
>> + if (IS_ERR(reply)) {
>> + err = PTR_ERR(reply);
>> + goto exit_free_meter;
>> + }
>> +
>> + ovs_lock();
>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>> + if (!dp) {
>> + err = -ENODEV;
>> + goto exit_unlock;
>> + }
>> +
>> + if (!a[OVS_METER_ATTR_ID]) {
>> + err = -ENODEV;
>> + goto exit_unlock;
>> + }
>> +
>> + meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>> +
>> + /* Cannot fail after this. */
>> + old_meter = lookup_meter(dp, meter_id);
>> + attach_meter(dp, meter);
>> + ovs_unlock();
>> +
> After the unlock, it is not safe to keep the ref to old_meter. better
> to release lock at the end. we could optimize it later if required.
I see a problem here: the old_meter has not been removed from the list before
unlock. The code should have been:
old_meter = lookup_meter(dp, meter_id);
detch_meter(dp, old_meter);
attach_meter(dp, meter);
ovs_unlock();
Do you still see a problem w.r.t. unlock() here? Once detch_meter() is called,
another thread should not have access to 'old_meter' any more. right?
>
>> + /* Build response with the meter_id and stats from
>> + * the old meter, if any.
>> + */
>> + failed = nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id);
>> + WARN_ON(failed);
>> + if (old_meter) {
>> + spin_lock_bh(&old_meter->lock);
>> + if (old_meter->keep_stats) {
>> + err = ovs_meter_cmd_reply_stats(reply, meter_id,
>> + old_meter);
>> + WARN_ON(err);
>> + }
>> + spin_unlock_bh(&old_meter->lock);
>> + ovs_meter_free(old_meter);
>> + }
>> +
>> + genlmsg_end(reply, ovs_reply_header);
>> + return genlmsg_reply(reply, info);
>> +
>> +exit_unlock:
>> + ovs_unlock();
>> + nlmsg_free(reply);
>> +exit_free_meter:
>> + kfree(meter);
>> + return err;
>> +}
>> +
> ....
>
>> +bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
>> + struct sw_flow_key *key, u32 meter_id)
>> +{
>> + struct dp_meter *meter;
>> + struct dp_meter_band *band;
>> + long long int now_ms = ktime_get_ns() / 1000 / 1000;
>> + long long int long_delta_ms;
>> + u32 delta_ms;
>> + u32 cost;
>> + int i, band_exceeded_max = -1;
>> + u32 band_exceeded_rate = 0;
>> +
>> + meter = lookup_meter(dp, meter_id);
>> + /* Do not drop the packet when there is no meter. */
>> + if (!meter)
>> + return false;
>> +
>> + /* Lock the meter while using it. */
>> + spin_lock(&meter->lock);
>> +
>> + long_delta_ms = (now_ms - meter->used); /* ms */
>> +
>> + /* Make sure delta_ms will not be too large, so that bucket will not
>> + * wrap around below.
>> + */
>> + delta_ms = (long_delta_ms > (long long int)meter->max_delta_t)
>> + ? meter->max_delta_t : (u32)long_delta_ms;
>> +
>> + /* Update meter statistics.
>> + */
>> + meter->used = now_ms;
>> + meter->stats.n_packets += 1;
>> + meter->stats.n_bytes += skb->len;
>> +
>> + /* Bucket rate is either in kilobits per second, or in packets per
>> + * second. We maintain the bucket in the units of either bits or
>> + * 1/1000th of a packet, correspondingly.
>> + * Then, when rate is multiplied with milliseconds, we get the
>> + * bucket units:
>> + * msec * kbps = bits, and
>> + * msec * packets/sec = 1/1000 packets.
>> + *
>> + * 'cost' is the number of bucket units in this packet.
>> + */
>> + cost = (meter->kbps) ? skb->len * 8 : 1000;
>> +
>> + /* Update all bands and find the one hit with the highest rate. */
>> + for (i = 0; i < meter->n_bands; ++i) {
>> + long long int max_bucket_size;
>> +
>> + band = &meter->bands[i];
>> + max_bucket_size = (band->burst_size + band->rate) * 1000;
>> +
>> + band->bucket += delta_ms * band->rate;
>> + if (band->bucket > max_bucket_size)
>> + band->bucket = max_bucket_size;
>> +
>> + if (band->bucket >= cost) {
>> + band->bucket -= cost;
>> + } else if (band->rate > band_exceeded_rate) {
>> + band_exceeded_rate = band->rate;
>> + band_exceeded_max = i;
>> + }
>> + }
>> +
>> + spin_unlock(&meter->lock);
>> +
>> + if (band_exceeded_max >= 0) {
>> + /* Update band statistics. */
>> + band = &meter->bands[band_exceeded_max];
>> + band->stats.n_packets += 1;
>> + band->stats.n_bytes += skb->len;
>> +
> Is it safe to do outside of the sipinlock?
Good catch, those should be covered by the same lock. Will fix.
>
>> + /* Drop band triggered, let the caller drop the 'skb'. */
>> + if (band->type == OVS_METER_BAND_TYPE_DROP)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
^ permalink raw reply
* Re: [net-next RFC 4/4] openvswitch: Add meter action support
From: Andy Zhou @ 2017-10-16 7:06 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <CAOrHB_AaUYb_s4Mp9gOBsH26P1CHcnEcLjPW8ZDeJxPCESP3pw@mail.gmail.com>
On Fri, Oct 13, 2017 at 5:13 PM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
>> Implements OVS kernel meter action support.
>>
>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>> ---
>> include/uapi/linux/openvswitch.h | 1 +
>> net/openvswitch/actions.c | 12 ++++++++++++
>> net/openvswitch/datapath.h | 1 +
>> net/openvswitch/flow_netlink.c | 6 ++++++
>> 4 files changed, 20 insertions(+)
>>
>> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> index 325049a129e4..11fe1a06cdd6 100644
>> --- a/include/uapi/linux/openvswitch.h
>> +++ b/include/uapi/linux/openvswitch.h
>> @@ -835,6 +835,7 @@ enum ovs_action_attr {
>> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
>> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
>> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
>> + OVS_ACTION_ATTR_METER, /* u32 meter ID. */
>>
>> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
>> * from userspace. */
>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> index a54a556fcdb5..4eb160ac5a27 100644
>> --- a/net/openvswitch/actions.c
>> +++ b/net/openvswitch/actions.c
>> @@ -1210,6 +1210,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> case OVS_ACTION_ATTR_POP_ETH:
>> err = pop_eth(skb, key);
>> break;
>> +
>> + case OVS_ACTION_ATTR_METER:
>> + if (ovs_meter_execute(dp, skb, key, nla_get_u32(a))) {
>> + consume_skb(skb);
>> + return 0;
>> + }
>> }
>>
>> if (unlikely(err)) {
>> @@ -1341,6 +1347,12 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> err = do_execute_actions(dp, skb, key,
>> acts->actions, acts->actions_len);
>>
>> + /* OVS action has dropped the packet, do not expose it
>> + * to the user.
>> + */
>> + if (err == -ENODATA)
>> + err = 0;
>> +
> I am not sure who is returning this error code?
Ah, this hunk was left over from experimenting with per band actions
list. Will remove. Thanks for catching this!
^ permalink raw reply
* using verifier to ensure a BPF program uses certain metadata?
From: Johannes Berg @ 2017-10-16 7:38 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, alexei.starovoitov, linux-wireless
Hi,
As we discussed in April already (it's really been that long...), I'd
wanted to allow using BPF to filter wireless monitor frames, to enable
new use cases and higher performance in monitoring. I have some code,
at
https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git/log/?h=bpf
which implements parts of this. It's still missing the TX status path
and perhaps associated metadata, but that part is easy.
The bigger "problem" is that we're going to be adding support for
devices that have 802.11->Ethernet conversion already in hardware, and
in that case the notion that the filter program will get an 802.11
header to look at is no longer right.
Now, most likely for the actual in-service monitoring we'll actually
have to reconstitute the 802.11 header on the fly (in pure monitoring
where nothing else is active, we can just disable the conversion), but
the filtering shouldn't really be reliant on that, since that's not the
cheapest thing to do.
The obvious idea around this is to add a metadata field (just a bit
really), something like "is_data_ethernet", saying that it was both a
data frame and is already converted to have an Ethernet header.
However, since these devices don't really exist yet for the vast
majority of people, I'm a bit afraid that we'll find later a lot of
code simply ignoring this field and looking at the "802.11" header,
which is then broken if it encounters an Ethernet header instead.
Are there lies my question: If we added a new callback to
bpf_verifier_ops (e.g. "post_verifier_check"), to be called after the
normal verification, and also added a context argument to
"is_valid_access" (*), we could easily track that this new metadata
field is accessed, and reject programs that don't access it at all.
Now, I realize that people could trivially just work around this in
their program if they wanted, but I think most will take the reminder
and just implement
if (ctx->is_data_ethernet)
return DROP_FRAME;
instead, since mostly data frames will not be very relevant to them.
What do you think?
johannes
(*) the context argument could just be a void **, and is_valid_access
can allocate memory if needed - in this case I'd probably just do
something like "return *is_valid_access ?: NULL;" and return something
like "(void *)1" for when the field in question was accessed, and then
just check that in "post_verifier_check".
^ permalink raw reply
* [patch net 0/4] net/sched: Fix a system panic when deleting filters
From: Chris Mi @ 2017-10-16 8:31 UTC (permalink / raw)
To: netdev; +Cc: jhs, lucasb, xiyou.wangcong, jiri, davem
If some filters share the same action, when deleting these filters,
system may panic. This patchset fixes this issue. And the test case
finding this issue is also integrated into tc test suite of selftests.
Chris Mi (4):
net/sched: Change tc_action refcnt and bindcnt to atomic
net/sched: Use action array instead of action list as parameter
selftests: Introduce a new script to generate tc batch file
selftests: Introduce a new test case to tc testsuite
include/net/act_api.h | 11 +-
net/sched/act_api.c | 124 +++++++++++++--------
net/sched/act_bpf.c | 4 +-
net/sched/act_connmark.c | 4 +-
net/sched/act_csum.c | 4 +-
net/sched/act_gact.c | 4 +-
net/sched/act_ife.c | 4 +-
net/sched/act_ipt.c | 4 +-
net/sched/act_mirred.c | 4 +-
net/sched/act_nat.c | 4 +-
net/sched/act_pedit.c | 4 +-
net/sched/act_police.c | 4 +-
net/sched/act_sample.c | 4 +-
net/sched/act_simple.c | 4 +-
net/sched/act_skbedit.c | 4 +-
net/sched/act_skbmod.c | 4 +-
net/sched/act_tunnel_key.c | 4 +-
net/sched/act_vlan.c | 4 +-
net/sched/cls_api.c | 18 +--
.../tc-testing/tc-tests/filters/tests.json | 23 +++-
tools/testing/selftests/tc-testing/tdc.py | 20 +++-
tools/testing/selftests/tc-testing/tdc_batch.py | 62 +++++++++++
tools/testing/selftests/tc-testing/tdc_config.py | 2 +
23 files changed, 222 insertions(+), 102 deletions(-)
create mode 100755 tools/testing/selftests/tc-testing/tdc_batch.py
--
1.8.3.1
^ permalink raw reply
* [patch net 2/4] net/sched: Use action array instead of action list as parameter
From: Chris Mi @ 2017-10-16 8:31 UTC (permalink / raw)
To: netdev; +Cc: jhs, lucasb, xiyou.wangcong, jiri, davem
In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com>
When destroying filters, actions should be destroyed first.
The pointers of each action are saved in an array. TC doesn't
use the array directly, but put all actions in a doubly linked
list and use that list as parameter.
There is no problem if each filter has its own actions. But if
some filters share the same action, when these filters are
destroyed, RCU callback fl_destroy_filter() may be called at the
same time. That means the same action's 'struct list_head list'
could be manipulated at the same time. It may point to an invalid
address so that system will panic.
This patch uses the action array directly to fix this issue.
Fixes commit in pre-git era.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Chris Mi <chrism@mellanox.com>
---
include/net/act_api.h | 7 ++--
net/sched/act_api.c | 103 +++++++++++++++++++++++++++++++-------------------
net/sched/cls_api.c | 18 +++------
3 files changed, 75 insertions(+), 53 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index a469ab6..081a313 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -148,16 +148,17 @@ static inline int tcf_idr_release(struct tc_action *a, bool bind)
int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops);
int tcf_unregister_action(struct tc_action_ops *a,
struct pernet_operations *ops);
-int tcf_action_destroy(struct list_head *actions, int bind);
+int tcf_action_destroy(struct tc_action **actions, int nr, int bind);
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
int nr_actions, struct tcf_result *res);
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions);
+ struct tc_action **actions, int *nr);
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind);
-int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
+int tcf_action_dump(struct sk_buff *skb, struct tc_action **actions, int nr,
+ int bind, int ref);
int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 9c0224d..391d560 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -513,13 +513,15 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
}
EXPORT_SYMBOL(tcf_action_exec);
-int tcf_action_destroy(struct list_head *actions, int bind)
+int tcf_action_destroy(struct tc_action **actions, int nr, int bind)
{
const struct tc_action_ops *ops;
- struct tc_action *a, *tmp;
+ struct tc_action *a;
int ret = 0;
+ int i;
- list_for_each_entry_safe(a, tmp, actions, list) {
+ for (i = 0; i < nr; i++) {
+ a = actions[i];
ops = a->ops;
ret = __tcf_idr_release(a, bind, true);
if (ret == ACT_P_DELETED)
@@ -568,14 +570,16 @@ int tcf_action_destroy(struct list_head *actions, int bind)
}
EXPORT_SYMBOL(tcf_action_dump_1);
-int tcf_action_dump(struct sk_buff *skb, struct list_head *actions,
+int tcf_action_dump(struct sk_buff *skb, struct tc_action **actions, int nr,
int bind, int ref)
{
struct tc_action *a;
- int err = -EINVAL;
struct nlattr *nest;
+ int err = -EINVAL;
+ int i;
- list_for_each_entry(a, actions, list) {
+ for (i = 0; i < nr; i++) {
+ a = actions[i];
nest = nla_nest_start(skb, a->order);
if (nest == NULL)
goto nla_put_failure;
@@ -700,10 +704,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
err = tcf_action_goto_chain_init(a, tp);
if (err) {
- LIST_HEAD(actions);
-
- list_add_tail(&a->list, &actions);
- tcf_action_destroy(&actions, bind);
+ tcf_action_destroy(&a, 1, bind);
return ERR_PTR(err);
}
}
@@ -720,23 +721,27 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
return ERR_PTR(err);
}
-static void cleanup_a(struct list_head *actions, int ovr)
+static void cleanup_a(struct tc_action **actions, int nr, int ovr)
{
struct tc_action *a;
+ int i;
if (!ovr)
return;
- list_for_each_entry(a, actions, list)
+ for (i = 0; i < nr; i++) {
+ a = actions[i];
atomic_dec(&a->tcfa_refcnt);
+ }
}
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions)
+ struct tc_action **actions, int *nr)
{
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+ int n = 0;
int err;
int i;
@@ -753,17 +758,19 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
act->order = i;
if (ovr)
atomic_inc(&act->tcfa_refcnt);
- list_add_tail(&act->list, actions);
+ actions[n++] = act;
}
+ *nr = n;
/* Remove the temp refcnt which was necessary to protect against
* destroying an existing action which was being replaced
*/
- cleanup_a(actions, ovr);
+ cleanup_a(actions, n, ovr);
return 0;
err:
- tcf_action_destroy(actions, bind);
+ tcf_action_destroy(actions, n, bind);
+ *nr = 0;
return err;
}
@@ -811,9 +818,9 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,
return -1;
}
-static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
- u32 portid, u32 seq, u16 flags, int event, int bind,
- int ref)
+static int tca_get_fill(struct sk_buff *skb, struct tc_action **actions,
+ int nr, u32 portid, u32 seq, u16 flags, int event,
+ int bind, int ref)
{
struct tcamsg *t;
struct nlmsghdr *nlh;
@@ -832,7 +839,7 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
if (nest == NULL)
goto out_nlmsg_trim;
- if (tcf_action_dump(skb, actions, bind, ref) < 0)
+ if (tcf_action_dump(skb, actions, nr, bind, ref) < 0)
goto out_nlmsg_trim;
nla_nest_end(skb, nest);
@@ -847,14 +854,14 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
static int
tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
- struct list_head *actions, int event)
+ struct tc_action **actions, int nr, int event)
{
struct sk_buff *skb;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
return -ENOBUFS;
- if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
+ if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, 0, event,
0, 0) <= 0) {
kfree_skb(skb);
return -EINVAL;
@@ -968,7 +975,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
}
static int
-tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+tcf_del_notify(struct net *net, struct nlmsghdr *n,
+ struct tc_action **actions, int nr,
u32 portid)
{
int ret;
@@ -978,14 +986,14 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
if (!skb)
return -ENOBUFS;
- if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
- 0, 1) <= 0) {
+ if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, 0,
+ RTM_DELACTION, 0, 1) <= 0) {
kfree_skb(skb);
return -EINVAL;
}
/* now do the delete */
- ret = tcf_action_destroy(actions, 0);
+ ret = tcf_action_destroy(actions, nr, 0);
if (ret < 0) {
kfree_skb(skb);
return ret;
@@ -1002,10 +1010,11 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
u32 portid, int event)
{
- int i, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
+ struct tc_action **actions;
struct tc_action *act;
- LIST_HEAD(actions);
+ int i, ret;
+ int nr = 0;
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, NULL);
if (ret < 0)
@@ -1018,6 +1027,11 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
return -EINVAL;
}
+ actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *),
+ GFP_KERNEL);
+ if (!actions)
+ return -ENOMEM;
+
for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
act = tcf_action_get_1(net, tb[i], n, portid);
if (IS_ERR(act)) {
@@ -1025,25 +1039,28 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
goto err;
}
act->order = i;
- list_add_tail(&act->list, &actions);
+ actions[nr++] = act;
}
if (event == RTM_GETACTION)
- ret = tcf_get_notify(net, portid, n, &actions, event);
+ ret = tcf_get_notify(net, portid, n, actions, nr, event);
else { /* delete */
- ret = tcf_del_notify(net, n, &actions, portid);
+ ret = tcf_del_notify(net, n, actions, nr, portid);
if (ret)
goto err;
+ kfree(actions);
return ret;
}
err:
if (event != RTM_GETACTION)
- tcf_action_destroy(&actions, 0);
+ tcf_action_destroy(actions, nr, 0);
+ kfree(actions);
return ret;
}
static int
-tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+tcf_add_notify(struct net *net, struct nlmsghdr *n,
+ struct tc_action **actions, int nr,
u32 portid)
{
struct sk_buff *skb;
@@ -1053,7 +1070,7 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
if (!skb)
return -ENOBUFS;
- if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
+ if (tca_get_fill(skb, actions, nr, portid, n->nlmsg_seq, n->nlmsg_flags,
RTM_NEWACTION, 0, 0) <= 0) {
kfree_skb(skb);
return -EINVAL;
@@ -1069,14 +1086,24 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
static int tcf_action_add(struct net *net, struct nlattr *nla,
struct nlmsghdr *n, u32 portid, int ovr)
{
+ struct tc_action **actions;
int ret = 0;
- LIST_HEAD(actions);
+ int nr;
- ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions);
+ actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *),
+ GFP_KERNEL);
+ if (!actions)
+ return -ENOMEM;
+
+ ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0,
+ actions, &nr);
if (ret)
- return ret;
+ goto out;
- return tcf_add_notify(net, n, &actions, portid);
+ ret = tcf_add_notify(net, n, actions, nr, portid);
+out:
+ kfree(actions);
+ return ret;
}
static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0b2219a..acaa0c6 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -879,8 +879,7 @@ void tcf_exts_destroy(struct tcf_exts *exts)
#ifdef CONFIG_NET_CLS_ACT
LIST_HEAD(actions);
- tcf_exts_to_list(exts, &actions);
- tcf_action_destroy(&actions, TCA_ACT_UNBIND);
+ tcf_action_destroy(exts->actions, exts->nr_actions, TCA_ACT_UNBIND);
kfree(exts->actions);
exts->nr_actions = 0;
#endif
@@ -905,17 +904,14 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
exts->actions[0] = act;
exts->nr_actions = 1;
} else if (exts->action && tb[exts->action]) {
- LIST_HEAD(actions);
- int err, i = 0;
+ int err;
err = tcf_action_init(net, tp, tb[exts->action],
rate_tlv, NULL, ovr, TCA_ACT_BIND,
- &actions);
+ exts->actions,
+ &exts->nr_actions);
if (err)
return err;
- list_for_each_entry(act, &actions, list)
- exts->actions[i++] = act;
- exts->nr_actions = i;
}
}
#else
@@ -961,14 +957,12 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
* tc data even if iproute2 was newer - jhs
*/
if (exts->type != TCA_OLD_COMPAT) {
- LIST_HEAD(actions);
-
nest = nla_nest_start(skb, exts->action);
if (nest == NULL)
goto nla_put_failure;
- tcf_exts_to_list(exts, &actions);
- if (tcf_action_dump(skb, &actions, 0, 0) < 0)
+ if (tcf_action_dump(skb, exts->actions,
+ exts->nr_actions, 0, 0) < 0)
goto nla_put_failure;
nla_nest_end(skb, nest);
} else if (exts->police) {
--
1.8.3.1
^ permalink raw reply related
* [patch net 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Chris Mi @ 2017-10-16 8:31 UTC (permalink / raw)
To: netdev; +Cc: jhs, lucasb, xiyou.wangcong, jiri, davem
In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com>
If many filters share the same action. That action's refcnt and bindcnt
could be manipulated by many RCU callbacks at the same time. This patch
makes these operations atomic.
Fixes commit in pre-git era.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Chris Mi <chrism@mellanox.com>
---
include/net/act_api.h | 4 ++--
net/sched/act_api.c | 21 +++++++++++----------
net/sched/act_bpf.c | 4 ++--
net/sched/act_connmark.c | 4 ++--
net/sched/act_csum.c | 4 ++--
net/sched/act_gact.c | 4 ++--
net/sched/act_ife.c | 4 ++--
net/sched/act_ipt.c | 4 ++--
net/sched/act_mirred.c | 4 ++--
net/sched/act_nat.c | 4 ++--
net/sched/act_pedit.c | 4 ++--
net/sched/act_police.c | 4 ++--
net/sched/act_sample.c | 4 ++--
net/sched/act_simple.c | 4 ++--
net/sched/act_skbedit.c | 4 ++--
net/sched/act_skbmod.c | 4 ++--
net/sched/act_tunnel_key.c | 4 ++--
net/sched/act_vlan.c | 4 ++--
18 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index b944e0eb..a469ab6 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -25,8 +25,8 @@ struct tc_action {
struct tcf_idrinfo *idrinfo;
u32 tcfa_index;
- int tcfa_refcnt;
- int tcfa_bindcnt;
+ atomic_t tcfa_refcnt;
+ atomic_t tcfa_bindcnt;
u32 tcfa_capab;
int tcfa_action;
struct tcf_t tcfa_tm;
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index da6fa82..9c0224d 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -88,12 +88,13 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool strict)
if (p) {
if (bind)
- p->tcfa_bindcnt--;
- else if (strict && p->tcfa_bindcnt > 0)
+ atomic_dec(&p->tcfa_bindcnt);
+ else if (strict && atomic_read(&p->tcfa_bindcnt) > 0)
return -EPERM;
- p->tcfa_refcnt--;
- if (p->tcfa_bindcnt <= 0 && p->tcfa_refcnt <= 0) {
+ atomic_dec(&p->tcfa_refcnt);
+ if (atomic_read(&p->tcfa_bindcnt) == 0 &&
+ atomic_read(&p->tcfa_refcnt) == 0) {
if (p->ops->cleanup)
p->ops->cleanup(p, bind);
tcf_idr_remove(p->idrinfo, p);
@@ -245,8 +246,8 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
if (index && p) {
if (bind)
- p->tcfa_bindcnt++;
- p->tcfa_refcnt++;
+ atomic_inc(&p->tcfa_bindcnt);
+ atomic_inc(&p->tcfa_refcnt);
*a = p;
return true;
}
@@ -274,9 +275,9 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
if (unlikely(!p))
return -ENOMEM;
- p->tcfa_refcnt = 1;
+ atomic_set(&p->tcfa_refcnt, 1);
if (bind)
- p->tcfa_bindcnt = 1;
+ atomic_set(&p->tcfa_bindcnt, 1);
if (cpustats) {
p->cpu_bstats = netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
@@ -727,7 +728,7 @@ static void cleanup_a(struct list_head *actions, int ovr)
return;
list_for_each_entry(a, actions, list)
- a->tcfa_refcnt--;
+ atomic_dec(&a->tcfa_refcnt);
}
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
@@ -751,7 +752,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
}
act->order = i;
if (ovr)
- act->tcfa_refcnt++;
+ atomic_inc(&act->tcfa_refcnt);
list_add_tail(&act->list, actions);
}
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index c0c707e..4ddf281 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -141,8 +141,8 @@ static int tcf_bpf_dump(struct sk_buff *skb, struct tc_action *act,
struct tcf_bpf *prog = to_bpf(act);
struct tc_act_bpf opt = {
.index = prog->tcf_index,
- .refcnt = prog->tcf_refcnt - ref,
- .bindcnt = prog->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&prog->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&prog->tcf_bindcnt) - bind,
.action = prog->tcf_action,
};
struct tcf_t tm;
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 10b7a88..d2cf658 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -153,8 +153,8 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
struct tc_connmark opt = {
.index = ci->tcf_index,
- .refcnt = ci->tcf_refcnt - ref,
- .bindcnt = ci->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&ci->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,
.action = ci->tcf_action,
.zone = ci->zone,
};
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 1c40caa..b9ca424 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -575,8 +575,8 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
.update_flags = p->update_flags,
.index = p->tcf_index,
.action = p->tcf_action,
- .refcnt = p->tcf_refcnt - ref,
- .bindcnt = p->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&p->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&p->tcf_bindcnt) - bind,
};
struct tcf_t t;
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index e29a48e..b1326b7 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -169,8 +169,8 @@ static int tcf_gact_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_gact *gact = to_gact(a);
struct tc_gact opt = {
.index = gact->tcf_index,
- .refcnt = gact->tcf_refcnt - ref,
- .bindcnt = gact->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&gact->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&gact->tcf_bindcnt) - bind,
.action = gact->tcf_action,
};
struct tcf_t t;
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 8ccd358..5cdcc87 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -551,8 +551,8 @@ static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bind,
struct tcf_ife_info *ife = to_ife(a);
struct tc_ife opt = {
.index = ife->tcf_index,
- .refcnt = ife->tcf_refcnt - ref,
- .bindcnt = ife->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&ife->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&ife->tcf_bindcnt) - bind,
.action = ife->tcf_action,
.flags = ife->flags,
};
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index d9e399a..02c14dc 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -277,8 +277,8 @@ static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
if (unlikely(!t))
goto nla_put_failure;
- c.bindcnt = ipt->tcf_bindcnt - bind;
- c.refcnt = ipt->tcf_refcnt - ref;
+ c.bindcnt = atomic_read(&ipt->tcf_bindcnt) - bind;
+ c.refcnt = atomic_read(&ipt->tcf_refcnt) - ref;
strcpy(t->u.user.name, ipt->tcfi_t->u.kernel.target->name);
if (nla_put(skb, TCA_IPT_TARG, ipt->tcfi_t->u.user.target_size, t) ||
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 416627c..aeeeb38 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -249,8 +249,8 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
struct tc_mirred opt = {
.index = m->tcf_index,
.action = m->tcf_action,
- .refcnt = m->tcf_refcnt - ref,
- .bindcnt = m->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&m->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&m->tcf_bindcnt) - bind,
.eaction = m->tcfm_eaction,
.ifindex = m->tcfm_ifindex,
};
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index c365d01..58fa1ae 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -256,8 +256,8 @@ static int tcf_nat_dump(struct sk_buff *skb, struct tc_action *a,
.index = p->tcf_index,
.action = p->tcf_action,
- .refcnt = p->tcf_refcnt - ref,
- .bindcnt = p->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&p->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&p->tcf_bindcnt) - bind,
};
struct tcf_t t;
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 491fe5de..27b9fea 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -391,8 +391,8 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
opt->nkeys = p->tcfp_nkeys;
opt->flags = p->tcfp_flags;
opt->action = p->tcf_action;
- opt->refcnt = p->tcf_refcnt - ref;
- opt->bindcnt = p->tcf_bindcnt - bind;
+ opt->refcnt = atomic_read(&p->tcf_refcnt) - ref;
+ opt->bindcnt = atomic_read(&p->tcf_bindcnt) - bind;
if (p->tcfp_keys_ex) {
tcf_pedit_key_ex_dump(skb, p->tcfp_keys_ex, p->tcfp_nkeys);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 3bb2ebf..d8a1ac6 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -272,8 +272,8 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
.action = police->tcf_action,
.mtu = police->tcfp_mtu,
.burst = PSCHED_NS2TICKS(police->tcfp_burst),
- .refcnt = police->tcf_refcnt - ref,
- .bindcnt = police->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&police->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&police->tcf_bindcnt) - bind,
};
struct tcf_t t;
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index ec986ae..64889a4 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -179,8 +179,8 @@ static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
struct tc_sample opt = {
.index = s->tcf_index,
.action = s->tcf_action,
- .refcnt = s->tcf_refcnt - ref,
- .bindcnt = s->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&s->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&s->tcf_bindcnt) - bind,
};
struct tcf_t t;
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index e7b57e5..ec7e397 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -148,8 +148,8 @@ static int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_defact *d = to_defact(a);
struct tc_defact opt = {
.index = d->tcf_index,
- .refcnt = d->tcf_refcnt - ref,
- .bindcnt = d->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&d->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
.action = d->tcf_action,
};
struct tcf_t t;
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 59949d6..2b05e56 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -172,8 +172,8 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_skbedit *d = to_skbedit(a);
struct tc_skbedit opt = {
.index = d->tcf_index,
- .refcnt = d->tcf_refcnt - ref,
- .bindcnt = d->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&d->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
.action = d->tcf_action,
};
struct tcf_t t;
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index b642ad3..ca1ddf9 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -201,8 +201,8 @@ static int tcf_skbmod_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_skbmod_params *p = rtnl_dereference(d->skbmod_p);
struct tc_skbmod opt = {
.index = d->tcf_index,
- .refcnt = d->tcf_refcnt - ref,
- .bindcnt = d->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&d->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&d->tcf_bindcnt) - bind,
.action = d->tcf_action,
};
struct tcf_t t;
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 30c9627..158d472 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -250,8 +250,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_tunnel_key_params *params;
struct tc_tunnel_key opt = {
.index = t->tcf_index,
- .refcnt = t->tcf_refcnt - ref,
- .bindcnt = t->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&t->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&t->tcf_bindcnt) - bind,
};
struct tcf_t tm;
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 16eb067..e2d7aab 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -208,8 +208,8 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
struct tcf_vlan *v = to_vlan(a);
struct tc_vlan opt = {
.index = v->tcf_index,
- .refcnt = v->tcf_refcnt - ref,
- .bindcnt = v->tcf_bindcnt - bind,
+ .refcnt = atomic_read(&v->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&v->tcf_bindcnt) - bind,
.action = v->tcf_action,
.v_action = v->tcfv_action,
};
--
1.8.3.1
^ permalink raw reply related
* [patch net 4/4] selftests: Introduce a new test case to tc testsuite
From: Chris Mi @ 2017-10-16 8:31 UTC (permalink / raw)
To: netdev; +Cc: jhs, lucasb, xiyou.wangcong, jiri, davem
In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com>
In this patchset, we fixed a tc bug. This patch adds the test case
that reproduces the bug. To run this test case, user should specify
an existing NIC device:
# sudo ./tdc.py -d enp4s0f0
This test case belongs to category "flower". If user doesn't specify
a NIC device, the test cases belong to "flower" will not be run.
In this test case, we create 1M filters and all filters share the same
action. When destroying all filters, kernel should not panic. It takes
about 18s to run it.
Signed-off-by: Chris Mi <chrism@mellanox.com>
---
.../tc-testing/tc-tests/filters/tests.json | 23 +++++++++++++++++++++-
tools/testing/selftests/tc-testing/tdc.py | 20 +++++++++++++++----
tools/testing/selftests/tc-testing/tdc_config.py | 2 ++
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
index c727b96..5fa02d8 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
@@ -17,5 +17,26 @@
"teardown": [
"$TC qdisc del dev $DEV1 ingress"
]
+ },
+ {
+ "id": "d052",
+ "name": "Add 1M filters with the same action",
+ "category": [
+ "filter",
+ "flower"
+ ],
+ "setup": [
+ "$TC qdisc add dev $DEV2 ingress",
+ "./tdc_batch.py $DEV2 $BATCH_FILE --share_action -n 1000000"
+ ],
+ "cmdUnderTest": "$TC -b $BATCH_FILE",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action gact",
+ "matchPattern": "action order 0: gact action drop.*index 1 ref 1000000 bind 1000000",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV2 ingress",
+ "/bin/rm $BATCH_FILE"
+ ]
}
-]
\ No newline at end of file
+]
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index cd61b78..5f11f5d 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -88,7 +88,7 @@ def prepare_env(cmdlist):
exit(1)
-def test_runner(filtered_tests):
+def test_runner(filtered_tests, args):
"""
Driver function for the unit tests.
@@ -105,6 +105,8 @@ def test_runner(filtered_tests):
for tidx in testlist:
result = True
tresult = ""
+ if "flower" in tidx["category"] and args.device == None:
+ continue
print("Test " + tidx["id"] + ": " + tidx["name"])
prepare_env(tidx["setup"])
(p, procout) = exec_cmd(tidx["cmdUnderTest"])
@@ -152,6 +154,10 @@ def ns_create():
exec_cmd(cmd, False)
cmd = 'ip -s $NS link set $DEV1 up'
exec_cmd(cmd, False)
+ cmd = 'ip link set $DEV2 netns $NS'
+ exec_cmd(cmd, False)
+ cmd = 'ip -s $NS link set $DEV2 up'
+ exec_cmd(cmd, False)
def ns_destroy():
@@ -211,7 +217,8 @@ def set_args(parser):
help='Execute the single test case with specified ID')
parser.add_argument('-i', '--id', action='store_true', dest='gen_id',
help='Generate ID numbers for new test cases')
- return parser
+ parser.add_argument('-d', '--device',
+ help='Execute the test case in flower category')
return parser
@@ -225,6 +232,8 @@ def check_default_settings(args):
if args.path != None:
NAMES['TC'] = args.path
+ if args.device != None:
+ NAMES['DEV2'] = args.device
if not os.path.isfile(NAMES['TC']):
print("The specified tc path " + NAMES['TC'] + " does not exist.")
exit(1)
@@ -381,14 +390,17 @@ def set_operation_mode(args):
if (len(alltests) == 0):
print("Cannot find a test case with ID matching " + target_id)
exit(1)
- catresults = test_runner(alltests)
+ catresults = test_runner(alltests, args)
print("All test results: " + "\n\n" + catresults)
elif (len(target_category) > 0):
+ if (target_category == "flower") and args.device == None:
+ print("Please specify a NIC device (-d) to run category flower")
+ exit(1)
if (target_category not in ucat):
print("Specified category is not present in this file.")
exit(1)
else:
- catresults = test_runner(testcases[target_category])
+ catresults = test_runner(testcases[target_category], args)
print("Category " + target_category + "\n\n" + catresults)
ns_destroy()
diff --git a/tools/testing/selftests/tc-testing/tdc_config.py b/tools/testing/selftests/tc-testing/tdc_config.py
index 0108737..b635251 100644
--- a/tools/testing/selftests/tc-testing/tdc_config.py
+++ b/tools/testing/selftests/tc-testing/tdc_config.py
@@ -12,6 +12,8 @@ NAMES = {
# Name of veth devices to be created for the namespace
'DEV0': 'v0p0',
'DEV1': 'v0p1',
+ 'DEV2': '',
+ 'BATCH_FILE': './batch.txt',
# Name of the namespace to use
'NS': 'tcut'
}
--
1.8.3.1
^ permalink raw reply related
* [patch net 3/4] selftests: Introduce a new script to generate tc batch file
From: Chris Mi @ 2017-10-16 8:31 UTC (permalink / raw)
To: netdev; +Cc: jhs, lucasb, xiyou.wangcong, jiri, davem
In-Reply-To: <1508142702-3312-1-git-send-email-chrism@mellanox.com>
# ./tdc_batch.py -h
usage: tdc_batch.py [-h] [-n NUMBER] [-o] [-s] [-p] device file
TC batch file generator
positional arguments:
device device name
file batch file name
optional arguments:
-h, --help show this help message and exit
-n NUMBER, --number NUMBER
how many lines in batch file
-o, --skip_sw skip_sw (offload), by default skip_hw
-s, --share_action all filters share the same action
-p, --prio all filters have different prio
Signed-off-by: Chris Mi <chrism@mellanox.com>
---
tools/testing/selftests/tc-testing/tdc_batch.py | 62 +++++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100755 tools/testing/selftests/tc-testing/tdc_batch.py
diff --git a/tools/testing/selftests/tc-testing/tdc_batch.py b/tools/testing/selftests/tc-testing/tdc_batch.py
new file mode 100755
index 0000000..707c6bf
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tdc_batch.py
@@ -0,0 +1,62 @@
+#!/usr/bin/python3
+
+"""
+tdc_batch.py - a script to generate TC batch file
+
+Copyright (C) 2017 Chris Mi <chrism@mellanox.com>
+"""
+
+import argparse
+
+parser = argparse.ArgumentParser(description='TC batch file generator')
+parser.add_argument("device", help="device name")
+parser.add_argument("file", help="batch file name")
+parser.add_argument("-n", "--number", type=int,
+ help="how many lines in batch file")
+parser.add_argument("-o", "--skip_sw",
+ help="skip_sw (offload), by default skip_hw",
+ action="store_true")
+parser.add_argument("-s", "--share_action",
+ help="all filters share the same action",
+ action="store_true")
+parser.add_argument("-p", "--prio",
+ help="all filters have different prio",
+ action="store_true")
+args = parser.parse_args()
+
+device = args.device
+file = open(args.file, 'w')
+
+number = 1
+if args.number:
+ number = args.number
+
+skip = "skip_hw"
+if args.skip_sw:
+ skip = "skip_sw"
+
+share_action = ""
+if args.share_action:
+ share_action = "index 1"
+
+prio = "prio 1"
+if args.prio:
+ prio = ""
+ if number > 0x4000:
+ number = 0x4000
+
+index = 0
+for i in range(0x100):
+ for j in range(0x100):
+ for k in range(0x100):
+ mac = ("%02x:%02x:%02x" % (i, j, k))
+ src_mac = "e4:11:00:" + mac
+ dst_mac = "e4:12:00:" + mac
+ cmd = ("filter add dev %s %s protocol ip parent ffff: flower %s "
+ "src_mac %s dst_mac %s action drop %s" %
+ (device, prio, skip, src_mac, dst_mac, share_action))
+ file.write("%s\n" % cmd)
+ index += 1
+ if index >= number:
+ file.close()
+ exit(0)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: move memcpy_to[from]_msg() from skbuff.h to socket.h
From: kbuild test robot @ 2017-10-16 8:49 UTC (permalink / raw)
To: yuan linyu; +Cc: kbuild-all, netdev, David S . Miller, yuan linyu
In-Reply-To: <1507947993-4728-1-git-send-email-cugyly@163.com>
[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]
Hi yuan,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/yuan-linyu/net-add-skb_memcpy_to-from-_msg/20171016-154618
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All error/warnings (new ones prefixed by >>):
In file included from include/uapi/linux/if.h:24:0,
from include/linux/inetdevice.h:7,
from net//ipv4/netfilter/ipt_MASQUERADE.c:13:
include/linux/socket.h: In function 'memcpy_from_msg':
>> include/linux/socket.h:59:63: error: 'EFAULT' undeclared (first use in this function)
return copy_from_iter_full(data, len, &msg->msg_iter) ? 0 : -EFAULT;
^
include/linux/socket.h:59:63: note: each undeclared identifier is reported only once for each function it appears in
include/linux/socket.h: In function 'memcpy_to_msg':
include/linux/socket.h:64:63: error: 'EFAULT' undeclared (first use in this function)
return copy_to_iter(data, len, &msg->msg_iter) == len ? 0 : -EFAULT;
^
--
In file included from include/uapi/linux/nfc.h:25:0,
from net/nfc/rawsock.c:25:
include/linux/socket.h: In function 'memcpy_from_msg':
>> include/linux/socket.h:59:63: error: 'EFAULT' undeclared (first use in this function)
return copy_from_iter_full(data, len, &msg->msg_iter) ? 0 : -EFAULT;
^
include/linux/socket.h:59:63: note: each undeclared identifier is reported only once for each function it appears in
include/linux/socket.h: In function 'memcpy_to_msg':
include/linux/socket.h:64:63: error: 'EFAULT' undeclared (first use in this function)
return copy_to_iter(data, len, &msg->msg_iter) == len ? 0 : -EFAULT;
^
In file included from include/uapi/linux/nfc.h:25:0,
from net/nfc/rawsock.c:25:
include/linux/socket.h: In function 'memcpy_from_msg':
>> include/linux/socket.h:60:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +/EFAULT +59 include/linux/socket.h
56
57 static inline int memcpy_from_msg(void *data, struct msghdr *msg, int len)
58 {
> 59 return copy_from_iter_full(data, len, &msg->msg_iter) ? 0 : -EFAULT;
> 60 }
61
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51658 bytes --]
^ permalink raw reply
* Re: [net-next 4/9] i40e: Enable 'channel' mode in mqprio for TC configs
From: Yunsheng Lin @ 2017-10-16 8:53 UTC (permalink / raw)
To: Jeff Kirsher, davem; +Cc: Amritha Nambiar, netdev, nhorman, sassmann, jogreene
In-Reply-To: <20171013215249.61286-5-jeffrey.t.kirsher@intel.com>
Hi, Jeff
On 2017/10/14 5:52, Jeff Kirsher wrote:
> From: Amritha Nambiar <amritha.nambiar@intel.com>
>
> The i40e driver is modified to enable the new mqprio hardware
> offload mode and factor the TCs and queue configuration by
> creating channel VSIs. In this mode, the priority to traffic
> class mapping and the user specified queue ranges are used
> to configure the traffic classes by setting the mode option to
> 'channel'.
>
> Example:
> map 0 0 0 0 1 2 2 3 queues 2@0 2@2 1@4 1@5\
> hw 1 mode channel
>
> qdisc mqprio 8038: root tc 4 map 0 0 0 0 1 2 2 3 0 0 0 0 0 0 0 0
> queues:(0:1) (2:3) (4:4) (5:5)
> mode:channel
> shaper:dcb
>
> The HW channels created are removed and all the queue configuration
> is set to default when the qdisc is detached from the root of the
> device.
>
> This patch also disables setting up channels via ethtool (ethtool -L)
> when the TCs are configured using mqprio scheduler.
>
> The patch also limits setting ethtool Rx flow hash indirection
> (ethtool -X eth0 equal N) to max queues configured via mqprio.
> The Rx flow hash indirection input through ethtool should be
> validated so that it is within in the queue range configured via
> tc/mqprio. The bound checking is achieved by reporting the current
> rss size to the kernel when queues are configured via mqprio.
>
> Example:
> map 0 0 0 1 0 2 3 0 queues 2@0 4@2 8@6 11@14\
> hw 1 mode channel
>
> Cannot set RX flow hash configuration: Invalid argument
>
> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 3 +
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 8 +-
> drivers/net/ethernet/intel/i40e/i40e_main.c | 457 +++++++++++++++++++------
> 3 files changed, 362 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index bde982541772..024c88474951 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -54,6 +54,7 @@
> #include <linux/clocksource.h>
> #include <linux/net_tstamp.h>
> #include <linux/ptp_clock_kernel.h>
> +#include <net/pkt_cls.h>
> #include "i40e_type.h"
> #include "i40e_prototype.h"
> #include "i40e_client.h"
> @@ -700,6 +701,7 @@ struct i40e_vsi {
> enum i40e_vsi_type type; /* VSI type, e.g., LAN, FCoE, etc */
> s16 vf_id; /* Virtual function ID for SRIOV VSIs */
>
> + struct tc_mqprio_qopt_offload mqprio_qopt; /* queue parameters */
> struct i40e_tc_configuration tc_config;
> struct i40e_aqc_vsi_properties_data info;
>
> @@ -725,6 +727,7 @@ struct i40e_vsi {
> u16 cnt_q_avail; /* num of queues available for channel usage */
> u16 orig_rss_size;
> u16 current_rss_size;
> + bool reconfig_rss;
>
> u16 next_base_queue; /* next queue to be used for channel setup */
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index afd3ca8d9851..72d5f2cdf419 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -2652,7 +2652,7 @@ static int i40e_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
>
> switch (cmd->cmd) {
> case ETHTOOL_GRXRINGS:
> - cmd->data = vsi->num_queue_pairs;
> + cmd->data = vsi->rss_size;
> ret = 0;
> break;
> case ETHTOOL_GRXFH:
> @@ -3897,6 +3897,12 @@ static int i40e_set_channels(struct net_device *dev,
> if (vsi->type != I40E_VSI_MAIN)
> return -EINVAL;
>
> + /* We do not support setting channels via ethtool when TCs are
> + * configured through mqprio
> + */
> + if (pf->flags & I40E_FLAG_TC_MQPRIO)
> + return -EINVAL;
> +
> /* verify they are not requesting separate vectors */
> if (!count || ch->rx_count || ch->tx_count)
> return -EINVAL;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index e23105bee6d1..e803aa1552c6 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -1588,6 +1588,170 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
> return 0;
> }
>
> +/**
> + * i40e_config_rss_aq - Prepare for RSS using AQ commands
> + * @vsi: vsi structure
> + * @seed: RSS hash seed
> + **/
[...]
> + * i40e_vsi_set_default_tc_config - set default values for tc configuration
> + * @vsi: the VSI being configured
> + **/
> +static void i40e_vsi_set_default_tc_config(struct i40e_vsi *vsi)
> +{
> + u16 qcount;
> + int i;
> +
> + /* Only TC0 is enabled */
> + vsi->tc_config.numtc = 1;
> + vsi->tc_config.enabled_tc = 1;
> + qcount = min_t(int, vsi->alloc_queue_pairs,
> + i40e_pf_get_max_q_per_tc(vsi->back));
> + for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
> + /* For the TC that is not enabled set the offset to to default
> + * queue and allocate one queue for the given TC.
> + */
> + vsi->tc_config.tc_info[i].qoffset = 0;
> + if (i == 0)
> + vsi->tc_config.tc_info[i].qcount = qcount;
> + else
> + vsi->tc_config.tc_info[i].qcount = 1;
> + vsi->tc_config.tc_info[i].netdev_tc = 0;
> + }
> +}
> +
> /**
> * i40e_setup_tc - configure multiple traffic classes
> * @netdev: net device to configure
> - * @tc: number of traffic classes to enable
> + * @type_data: tc offload data
> **/
> -static int i40e_setup_tc(struct net_device *netdev, u8 tc)
> +static int i40e_setup_tc(struct net_device *netdev, void *type_data)
> {
> + struct tc_mqprio_qopt_offload *mqprio_qopt = type_data;
> struct i40e_netdev_priv *np = netdev_priv(netdev);
> struct i40e_vsi *vsi = np->vsi;
> struct i40e_pf *pf = vsi->back;
> - u8 enabled_tc = 0;
> + u8 enabled_tc = 0, num_tc, hw;
> + bool need_reset = false;
> int ret = -EINVAL;
> + u16 mode;
> int i;
>
> - /* Check if DCB enabled to continue */
> - if (!(pf->flags & I40E_FLAG_DCB_ENABLED)) {
> - netdev_info(netdev, "DCB is not enabled for adapter\n");
> - goto exit;
> + num_tc = mqprio_qopt->qopt.num_tc;
> + hw = mqprio_qopt->qopt.hw;
> + mode = mqprio_qopt->mode;
> + if (!hw) {
When stack call the ndo_setup_tc, then qopt.hw is always non-zero, Can you
tell me why you need to check for this?
Thanks,
Yunsheng Lin
> + pf->flags &= ~I40E_FLAG_TC_MQPRIO;
> + memcpy(&vsi->mqprio_qopt, mqprio_qopt, sizeof(*mqprio_qopt));
> + goto config_tc;
> }
>
> /* Check if MFP enabled */
> if (pf->flags & I40E_FLAG_MFP_ENABLED) {
> - netdev_info(netdev, "Configuring TC not supported in MFP mode\n");
> - goto exit;
> + netdev_info(netdev,
> + "Configuring TC not supported in MFP mode\n");
> + return ret;
> }
> + switch (mode) {
> + case TC_MQPRIO_MODE_DCB:
> + pf->flags &= ~I40E_FLAG_TC_MQPRIO;
>
> - /* Check whether tc count is within enabled limit */
> - if (tc > i40e_pf_get_num_tc(pf)) {
> - netdev_info(netdev, "TC count greater than enabled on link for adapter\n");
> - goto exit;
> + /* Check if DCB enabled to continue */
> + if (!(pf->flags & I40E_FLAG_DCB_ENABLED)) {
> + netdev_info(netdev,
> + "DCB is not enabled for adapter\n");
> + return ret;
> + }
> +
> + /* Check whether tc count is within enabled limit */
> + if (num_tc > i40e_pf_get_num_tc(pf)) {
> + netdev_info(netdev,
> + "TC count greater than enabled on link for adapter\n");
> + return ret;
> + }
> + break;
> + case TC_MQPRIO_MODE_CHANNEL:
> + if (pf->flags & I40E_FLAG_DCB_ENABLED) {
> + netdev_info(netdev,
> + "Full offload of TC Mqprio options is not supported when DCB is enabled\n");
> + return ret;
> + }
> + if (!(pf->flags & I40E_FLAG_MSIX_ENABLED))
> + return ret;
> + ret = i40e_validate_mqprio_qopt(vsi, mqprio_qopt);
> + if (ret)
> + return ret;
> + memcpy(&vsi->mqprio_qopt, mqprio_qopt,
> + sizeof(*mqprio_qopt));
> + pf->flags |= I40E_FLAG_TC_MQPRIO;
> + pf->flags &= ~I40E_FLAG_DCB_ENABLED;
> + break;
> + default:
> + return -EINVAL;
> }
>
> +config_tc:
> /* Generate TC map for number of tc requested */
> - for (i = 0; i < tc; i++)
> + for (i = 0; i < num_tc; i++)
> enabled_tc |= BIT(i);
>
> /* Requesting same TC configuration as already enabled */
> - if (enabled_tc == vsi->tc_config.enabled_tc)
> + if (enabled_tc == vsi->tc_config.enabled_tc &&
> + mode != TC_MQPRIO_MODE_CHANNEL)
> return 0;
>
> /* Quiesce VSI queues */
> i40e_quiesce_vsi(vsi);
>
> + if (!hw && !(pf->flags & I40E_FLAG_TC_MQPRIO))
> + i40e_remove_queue_channels(vsi);
> +
> /* Configure VSI for enabled TCs */
> ret = i40e_vsi_config_tc(vsi, enabled_tc);
> if (ret) {
> netdev_info(netdev, "Failed configuring TC for VSI seid=%d\n",
> vsi->seid);
> + need_reset = true;
> goto exit;
> }
>
> @@ -6272,11 +6595,18 @@ static int i40e_setup_tc(struct net_device *netdev, u8 tc)
> if (ret) {
> netdev_info(netdev,
> "Failed configuring queue channels\n");
> + need_reset = true;
> goto exit;
> }
> }
>
> exit:
> + /* Reset the configuration data to defaults, only TC0 is enabled */
> + if (need_reset) {
> + i40e_vsi_set_default_tc_config(vsi);
> + need_reset = false;
> + }
> +
> /* Unquiesce VSI */
> i40e_unquiesce_vsi(vsi);
> return ret;
> @@ -6285,14 +6615,10 @@ static int i40e_setup_tc(struct net_device *netdev, u8 tc)
> static int __i40e_setup_tc(struct net_device *netdev, enum tc_setup_type type,
> void *type_data)
> {
> - struct tc_mqprio_qopt *mqprio = type_data;
> -
> if (type != TC_SETUP_MQPRIO)
> return -EOPNOTSUPP;
>
> - mqprio->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
> -
> - return i40e_setup_tc(netdev, mqprio->num_tc);
> + return i40e_setup_tc(netdev, type_data);
> }
>
> /**
> @@ -9153,45 +9479,6 @@ static int i40e_setup_misc_vector(struct i40e_pf *pf)
> return err;
> }
>
> -/**
> - * i40e_config_rss_aq - Prepare for RSS using AQ commands
> - * @vsi: vsi structure
> - * @seed: RSS hash seed
> - **/
> -static int i40e_config_rss_aq(struct i40e_vsi *vsi, const u8 *seed,
> - u8 *lut, u16 lut_size)
> -{
> - struct i40e_pf *pf = vsi->back;
> - struct i40e_hw *hw = &pf->hw;
> - int ret = 0;
> -
> - if (seed) {
> - struct i40e_aqc_get_set_rss_key_data *seed_dw =
> - (struct i40e_aqc_get_set_rss_key_data *)seed;
> - ret = i40e_aq_set_rss_key(hw, vsi->id, seed_dw);
> - if (ret) {
> - dev_info(&pf->pdev->dev,
> - "Cannot set RSS key, err %s aq_err %s\n",
> - i40e_stat_str(hw, ret),
> - i40e_aq_str(hw, hw->aq.asq_last_status));
> - return ret;
> - }
> - }
> - if (lut) {
> - bool pf_lut = vsi->type == I40E_VSI_MAIN ? true : false;
> -
> - ret = i40e_aq_set_rss_lut(hw, vsi->id, pf_lut, lut, lut_size);
> - if (ret) {
> - dev_info(&pf->pdev->dev,
> - "Cannot set RSS lut, err %s aq_err %s\n",
> - i40e_stat_str(hw, ret),
> - i40e_aq_str(hw, hw->aq.asq_last_status));
> - return ret;
> - }
> - }
> - return ret;
> -}
> -
> /**
> * i40e_get_rss_aq - Get RSS keys and lut by using AQ commands
> * @vsi: Pointer to vsi structure
> @@ -9238,46 +9525,6 @@ static int i40e_get_rss_aq(struct i40e_vsi *vsi, const u8 *seed,
> return ret;
> }
>
> -/**
> - * i40e_vsi_config_rss - Prepare for VSI(VMDq) RSS if used
> - * @vsi: VSI structure
> - **/
> -static int i40e_vsi_config_rss(struct i40e_vsi *vsi)
> -{
> - u8 seed[I40E_HKEY_ARRAY_SIZE];
> - struct i40e_pf *pf = vsi->back;
> - u8 *lut;
> - int ret;
> -
> - if (!(pf->hw_features & I40E_HW_RSS_AQ_CAPABLE))
> - return 0;
> -
> - if (!vsi->rss_size)
> - vsi->rss_size = min_t(int, pf->alloc_rss_size,
> - vsi->num_queue_pairs);
> - if (!vsi->rss_size)
> - return -EINVAL;
> -
> - lut = kzalloc(vsi->rss_table_size, GFP_KERNEL);
> - if (!lut)
> - return -ENOMEM;
> - /* Use the user configured hash keys and lookup table if there is one,
> - * otherwise use default
> - */
> - if (vsi->rss_lut_user)
> - memcpy(lut, vsi->rss_lut_user, vsi->rss_table_size);
> - else
> - i40e_fill_rss_lut(pf, lut, vsi->rss_table_size, vsi->rss_size);
> - if (vsi->rss_hkey_user)
> - memcpy(seed, vsi->rss_hkey_user, I40E_HKEY_ARRAY_SIZE);
> - else
> - netdev_rss_key_fill((void *)seed, I40E_HKEY_ARRAY_SIZE);
> - ret = i40e_config_rss_aq(vsi, seed, lut, vsi->rss_table_size);
> - kfree(lut);
> -
> - return ret;
> -}
> -
> /**
> * i40e_config_rss_reg - Configure RSS keys and lut by writing registers
> * @vsi: Pointer to vsi structure
>
^ permalink raw reply
* RE: [PATCH net-next v2 2/4] net: dsa: mv88e6060: setup random mac address
From: David Laight @ 2017-10-16 9:02 UTC (permalink / raw)
To: 'Woojung.Huh@microchip.com',
vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
davem@davemloft.net, f.fainelli@gmail.com, andrew@lunn.ch
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B51650@CHN-SV-EXMX02.mchp-main.com>
From: Woojung.Huh@microchip.com
> Sent: 13 October 2017 18:59
> > >> > + REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 9) |
> > >> addr[1]);
> > >>
> > >> Is that supposed to be 9 ?
> > >
> > > Looks like it.
> > > Check
> > http://www.marvell.com/switching/assets/marvell_linkstreet_88E6060_data
> > sheet.pdf
> >
> > Hum, David is correct, there is a bug in the driver which needs to be
> > addressed first. MAC address bit 40 is addr[0] & 0x1, thus we must
> > shift byte 0 by 8 and mask it against 0xfe.
> >
> > I'll respin this serie including a fix for both net and net-next.
>
> Yes, you are right. Missed description about bit 40.
Since they are all random numbers it doesn't matter.
Actually isn't this just masking off the non-unicast bit?
So it won't be set in the data - and so doesn't need masking.
David
^ permalink raw reply
* RE: [PATCH net-next v3 3/5] net: dsa: mv88e6060: setup random mac address
From: David Laight @ 2017-10-16 9:05 UTC (permalink / raw)
To: 'Vivien Didelot', netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, Andrew Lunn
In-Reply-To: <20171013181809.14627-4-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot
> Sent: 13 October 2017 19:18
> As for mv88e6xxx, setup the switch from within the mv88e6060 driver with
> a random MAC address, and remove the .set_addr implementation.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> drivers/net/dsa/mv88e6060.c | 43 ++++++++++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
> index d64be2b83d3c..6173be889d95 100644
> --- a/drivers/net/dsa/mv88e6060.c
> +++ b/drivers/net/dsa/mv88e6060.c
> @@ -9,6 +9,7 @@
> */
>
> #include <linux/delay.h>
> +#include <linux/etherdevice.h>
> #include <linux/jiffies.h>
> #include <linux/list.h>
> #include <linux/module.h>
> @@ -188,6 +189,27 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
> return 0;
> }
>
> +static int mv88e6060_setup_addr(struct dsa_switch *ds)
> +{
> + u8 addr[ETH_ALEN];
> + u16 val;
> +
> + eth_random_addr(addr);
> +
> + val = addr[0] << 8 | addr[1];
> +
> + /* The multicast bit is always transmitted as a zero, so the switch uses
> + * bit 8 for "DiffAddr", where 0 means all ports transmit the same SA.
> + */
> + val &= 0xfeff;
The comment is probably ok, but the mask isn't needed.
eth_randmon_addr() won't return and address with the multicast bit set.
David
^ permalink raw reply
* RE: [next-queue PATCH v7 4/6] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: David Laight @ 2017-10-16 9:14 UTC (permalink / raw)
To: 'Ivan Khoronzhuk', Vinicius Costa Gomes
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
andre.guedes@intel.com, ivan.briano@intel.com,
jesus.sanchez-palencia@intel.com, boon.leong.ong@intel.com,
richardcochran@gmail.com, henrik@austad.us, levipearson@gmail.com,
rodney.cummings@ni.com
In-Reply-To: <20171013195924.GA19439@khorivan>
From: Ivan Khoronzhuk
> Sent: 13 October 2017 20:59
> On Thu, Oct 12, 2017 at 05:40:03PM -0700, Vinicius Costa Gomes wrote:
> > This queueing discipline implements the shaper algorithm defined by
> > the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
> >
> > It's primary usage is to apply some bandwidth reservation to user
> > defined traffic classes, which are mapped to different queues via the
> > mqprio qdisc.
> >
> > Only a simple software implementation is added for now.
> >
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> > Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> > ---
> > include/uapi/linux/pkt_sched.h | 18 +++
> > net/sched/Kconfig | 11 ++
> > net/sched/Makefile | 1 +
> > net/sched/sch_cbs.c | 314 +++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 344 insertions(+)
> > create mode 100644 net/sched/sch_cbs.c
> >
> > diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> > index 099bf5528fed..41e349df4bf4 100644
> > --- a/include/uapi/linux/pkt_sched.h
> > +++ b/include/uapi/linux/pkt_sched.h
> > @@ -871,4 +871,22 @@ struct tc_pie_xstats {
> > __u32 maxq; /* maximum queue size */
> > __u32 ecn_mark; /* packets marked with ecn*/
> > };
> > +
> > +/* CBS */
> > +struct tc_cbs_qopt {
> > + __u8 offload;
You probably don't want unnamed padding in a uapi structure.
> > + __s32 hicredit;
> > + __s32 locredit;
> > + __s32 idleslope;
> > + __s32 sendslope;
> > +};
> > +
> > +enum {
> > + TCA_CBS_UNSPEC,
> > + TCA_CBS_PARMS,
> > + __TCA_CBS_MAX,
> > +};
> > +
> > +#define TCA_CBS_MAX (__TCA_CBS_MAX - 1)
Why not:
TCA_CBS_PARMS,
TCA_CBS_NEXT,
TCA_CBS_MAX = TCA_CBS_NEXT - 1,
...
David
^ permalink raw reply
* [PATCH net] net/sched: cls_flower: Set egress_dev mark when calling into the HW driver
From: Or Gerlitz @ 2017-10-16 9:19 UTC (permalink / raw)
To: David S. Miller; +Cc: Jiri Pirko, netdev, mlxsw, Or Gerlitz, Roi Dayan
Commit 7091d8c '(net/sched: cls_flower: Add offload support using egress
Hardware device') made sure (when fl_hw_replace_filter is called) to put
the egress_dev mark on persisent structure instance. Hence, following calls
into the HW driver for stats and deletion will note it and act accordingly.
With commit de4784ca030f this property is lost and hence when called,
the HW driver failes to operate (stats, delete) on the offloaded flow.
Fix it by setting the egress_dev flag whenever the ingress device is
different from the hw device since this is exactly the condition under
which we're calling into the HW driver through the egress port net-device.
Fixes: de4784ca030f ('net: sched: get rid of struct tc_to_netdev')
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Roi Dayan <roid@mellanox.com>
---
Hi Dave, the bug was introduced in 4.14-rc1 but later the related
code was changed in net-next, hence the fix must not go to net-next, Or.
net/sched/cls_flower.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index d230cb4..b480d7c 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -234,6 +234,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
tc_cls_common_offload_init(&cls_flower.common, tp);
cls_flower.command = TC_CLSFLOWER_DESTROY;
cls_flower.cookie = (unsigned long) f;
+ cls_flower.egress_dev = f->hw_dev != tp->q->dev_queue->dev;
dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER, &cls_flower);
}
@@ -289,6 +290,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
cls_flower.command = TC_CLSFLOWER_STATS;
cls_flower.cookie = (unsigned long) f;
cls_flower.exts = &f->exts;
+ cls_flower.egress_dev = f->hw_dev != tp->q->dev_queue->dev;
dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSFLOWER,
&cls_flower);
--
2.3.7
^ permalink raw reply related
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