* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 8:14 UTC (permalink / raw)
To: Arad, Ronen
Cc: Fastabend, John R, Roopa Prabhu, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505DC9956@ORSMSX101.amr.corp.intel.com>
Thu, Dec 18, 2014 at 11:43:06PM CET, ronen.arad@intel.com wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>Behalf Of John Fastabend
>>Sent: Thursday, December 18, 2014 9:21 PM
>>To: Roopa Prabhu; Varlese, Marco
>>Cc: netdev@vger.kernel.org; Thomas Graf; Jiri Pirko; sfeldma@gmail.com; linux-
>>kernel@vger.kernel.org
>>Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port
>>configuration
>>
>>On 12/18/2014 10:14 AM, Roopa Prabhu wrote:
>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote:
>>>> Removed unnecessary content for ease of reading...
>>>>
>>>>>>>>>> +/* Switch Port Attributes section */
>>>>>>>>>> +
>>>>>>>>>> +enum {
>>>>>>>>>> + IFLA_ATTR_UNSPEC,
>>>>>>>>>> + IFLA_ATTR_LEARNING,
>>>>>>>>> Any reason you want learning here ?. This is covered as part of
>>>>>>>>> the bridge setlink attributes.
>>>>>>>>>
>>>>>>>> Yes, because the user may _not_ want to go through a bridge
>>>>>>>> interface
>>>>>>> necessarily.
>>>>>>> But, the bridge setlink/getlink interface was changed to accommodate
>>>>> 'self'
>>>>>>> for exactly such cases.
>>>>>>> I kind of understand your case for the other attributes (these are
>>>>>>> per port settings that switch asics provide).
>>>>>>>
>>>>>>> However, i don't understand the reason to pull in bridge attributes
>>here.
>>>>>>>
>>>>>> Maybe, I am missing something so you might help. The learning attribute -
>>>>> in my case - it is like all other attributes: a port attribute (as you
>>said, port
>>>>> settings that the switch provides per port).
>>>>>> So, what I was saying is "why the user shall go through a bridge to
>>configure
>>>>> the learning attribute"? From my perspective, it is as any other attribute
>>and
>>>>> as such configurable on the port.
>>>>>
>>>>> Thinking about this some more, i don't see why any of these attributes
>>>>> (except loopback. I dont understand the loopback attribute) cant be part
>>of
>>>>> the birdge port attributes.
>>>>>
>>>>> With this we will end up adding l2 attributes in two places: the general
>>link
>>>>> attributes and bridge attributes.
>>>>>
>>>>> And since we have gone down the path of using ndo_bridge_setlink/getlink
>>>>> with 'self'....we should stick to that for all l2 attributes.
>>>>>
>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same set
>>of
>>>>> attributes but support both cases where the user wants to go through the
>>>>> bridge driver or directly to the switch port driver. So, you are not
>>really going
>>>>> through the bridge driver if you use 'self' and
>>ndo_bridge_setlink/getlink.
>>>>>
>>
>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch
>>>> was that your patch and mine were supplementary ("I think Roopa's
>>>> patches are supplementary. Not all switchdev users will be backed
>>>> with a Linux Bridge. I therefore welcome your patches very
>>>> much")... I also understood by others that the patch made sense for
>>>> the same reason. I simply do not understand why these attributes
>>>> (and maybe others in the future) could not be configured directly
>>>> on a standard port but have to go through a bridge.
>>>>
>>> ok, i am very confused in that case. The whole moving of bridge
>>> attributes from the bridge driver to rtnetlink.c was to make the
>>> bridge attributes accessible to any driver who wants to set l2/bridge
>>> attributes on their switch ports. So, its unclear to me why we are
>>> doing this parallel thing again. This move to rtnetlink.c was done
>>> during the recent rocker support. so, maybe scott/jiri can elaborate
>>> more.
>>
>>
>>Not sure if this will add to the confusion or help. But you do not
>>need to have the bridge.ko loaded or netdev's attached to a bridge
>>to use the setlink/getlink ndo ops and netlink messages.
>
>No you don't need bridge.ko to implement ndo_bridge_setlink/getlink. Rtnetlink invokes those ndos from code which does not depend on CONFIG_BRIDGE or the presence of bridge.ko.
>Calling some bridge exported functions such as br_fdb_external_learn_add/del requires the presence of bridge.ko and it only makes sense when the switch port device is enslaved to a bridge.
Note I plan to change br_fdb_external_learn_add/del to (semi-)generic
notifier very soon.
>
>>
>>This was intentionally done. Its already used with NIC devices to
>>configure embedded bridge settings such as VEB/VEPA.
>>
>>I think I'm just repeating Roopa though.
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 8:25 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <549367CC.2080307@cumulusnetworks.com>
Fri, Dec 19, 2014 at 12:48:28AM CET, roopa@cumulusnetworks.com wrote:
>On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>>
>>On 12/18/2014 3:07 PM, Roopa Prabhu wrote:
>>>On 12/18/14, 11:21 AM, John Fastabend wrote:
>>>>On 12/18/2014 10:14 AM, Roopa Prabhu wrote:
>>>>>On 12/18/14, 10:02 AM, Varlese, Marco wrote:
>>>>>>Removed unnecessary content for ease of reading...
>>>>>>
>>>>>>>>>>>>+/* Switch Port Attributes section */
>>>>>>>>>>>>+
>>>>>>>>>>>>+enum {
>>>>>>>>>>>>+ IFLA_ATTR_UNSPEC,
>>>>>>>>>>>>+ IFLA_ATTR_LEARNING,
>>>>>>>>>>>Any reason you want learning here ?. This is covered as part of
>>>>>>>>>>>the bridge setlink attributes.
>>>>>>>>>>>
>>>>>>>>>>Yes, because the user may _not_ want to go through a bridge
>>>>>>>>>>interface
>>>>>>>>>necessarily.
>>>>>>>>>But, the bridge setlink/getlink interface was changed to
>>>>>>>>>accommodate
>>>>>>>'self'
>>>>>>>>>for exactly such cases.
>>>>>>>>>I kind of understand your case for the other attributes (these are
>>>>>>>>>per port settings that switch asics provide).
>>>>>>>>>
>>>>>>>>>However, i don't understand the reason to pull in bridge
>>>>>>>>>attributes here.
>>>>>>>>>
>>>>>>>>Maybe, I am missing something so you might help. The learning
>>>>>>>>attribute -
>>>>>>>in my case - it is like all other attributes: a port attribute
>>>>>>>(as you said, port
>>>>>>>settings that the switch provides per port).
>>>>>>>>So, what I was saying is "why the user shall go through a
>>>>>>>>bridge to configure
>>>>>>>the learning attribute"? From my perspective, it is as any other
>>>>>>>attribute and
>>>>>>>as such configurable on the port.
>>>>>>>
>>>>>>>Thinking about this some more, i don't see why any of these
>>>>>>>attributes
>>>>>>>(except loopback. I dont understand the loopback attribute) cant
>>>>>>>be part of
>>>>>>>the birdge port attributes.
>>>>>>>
>>>>>>>With this we will end up adding l2 attributes in two places: the
>>>>>>>general link
>>>>>>>attributes and bridge attributes.
>>>>>>>
>>>>>>>And since we have gone down the path of using
>>>>>>>ndo_bridge_setlink/getlink
>>>>>>>with 'self'....we should stick to that for all l2 attributes.
>>>>>>>
>>>>>>>The idea of overloading ndo_bridge_set/getlink, was to have the
>>>>>>>same set of
>>>>>>>attributes but support both cases where the user wants to go
>>>>>>>through the
>>>>>>>bridge driver or directly to the switch port driver. So, you are
>>>>>>>not really going
>>>>>>>through the bridge driver if you use 'self' and
>>>>>>>ndo_bridge_setlink/getlink.
>>>>>>>
>>>>>>Roopa, one of the comments I got from Thomas Graf on my v1 patch
>>>>>>was that your patch and mine were supplementary ("I think Roopa's
>>>>>>patches are supplementary. Not all switchdev users will be backed
>>>>>>with a Linux Bridge. I therefore welcome your patches very
>>>>>>much")... I also understood by others that the patch made sense for
>>>>>>the same reason. I simply do not understand why these attributes
>>>>>>(and maybe others in the future) could not be configured directly
>>>>>>on a standard port but have to go through a bridge.
>>>>>>
>>>>>ok, i am very confused in that case. The whole moving of bridge
>>>>>attributes from the bridge driver to rtnetlink.c was to make the
>>>>>bridge attributes accessible to any driver who wants to set l2/bridge
>>>>>attributes on their switch ports. So, its unclear to me why we are
>>>>>doing this parallel thing again. This move to rtnetlink.c was done
>>>>>during the recent rocker support. so, maybe scott/jiri can elaborate
>>>>>more.
>>>>
>>>>Not sure if this will add to the confusion or help. But you do not
>>>>need to have the bridge.ko loaded or netdev's attached to a bridge
>>>>to use the setlink/getlink ndo ops and netlink messages.
>>>>
>>>>This was intentionally done. Its already used with NIC devices to
>>>>configure embedded bridge settings such as VEB/VEPA.
>>>
>>>that helps my case, thanks.
>>
>>So the user interface to set/get the per-port attributes will be via
>>'bridge', not 'ip'
>>
>> bridge link set dev sw0p1 port_attr bcast_flooding 1 self
>> bridge link get dev sw0p1 port_attr bcast_flooding self
>
>yes, l2 attributes.
>>
>>We also need an interface to set per-switch attributes. Can this work?
>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>where sw0 is a bridge representing the hardware switch.
How can a bridge represent the hardware switch? What if you have 2 bridge
on ports of the same hardware switch. Or what if you have a bridge with
couple of ports from one hardware switch and couple of ports from second
hardware switch. Or else, you can have a misxture of switch ports and
other devices (taps for example). Bridge->hardware switch does not map
1:1.
If you want to set any per switch attributes, just use one of the ports
as a "gateway". port->hardware switch mapping is strict, so you should
be okay doing that. There are no ndos for this purpose at the moment,
but they can be easily added.
>
>Not today. We discussed this @ LPC, and one way to do this would be to have a
>device
>representing the switch asic. This is in the works.
>
Yep, I still do not find a need to introduce some new device class. I
think it would be just confusing. Example.
br0 --- sw0p0
--- sw0p1
--- sw0p2
br1 --- sw0p3
--- sw0p4
--- sw0p5
and in paralel, without any connection you would have some mysterious
device "sw0" which would do something, not clear why.
Btw this device would not be netdev, so it could not be used for
anything netdevs can be currently used for.
Yes, I still might be missing something, for sure. Nobody told me what yet.
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 8:27 UTC (permalink / raw)
To: B Viswanath
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+pFwJyfg1+=+tXUpon0Kket56HJ=9BR3XohPhZnaX7hfPgpg@mail.gmail.com>
Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>>>
>>>
>>> On 12/18/2014 3:07 PM, Roopa Prabhu wrote:
>>>>
>>>> On 12/18/14, 11:21 AM, John Fastabend wrote:
>>>>>
>>>>> On 12/18/2014 10:14 AM, Roopa Prabhu wrote:
>>>>>>
>>>>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote:
>>>>>>>
>>>>>>> Removed unnecessary content for ease of reading...
>>>>>>>
>>>>>>>>>>>>> +/* Switch Port Attributes section */
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +enum {
>>>>>>>>>>>>> + IFLA_ATTR_UNSPEC,
>>>>>>>>>>>>> + IFLA_ATTR_LEARNING,
>>>>>>>>>>>>
>>>>>>>>>>>> Any reason you want learning here ?. This is covered as part of
>>>>>>>>>>>> the bridge setlink attributes.
>>>>>>>>>>>>
>>>>>>>>>>> Yes, because the user may _not_ want to go through a bridge
>>>>>>>>>>> interface
>>>>>>>>>>
>>>>>>>>>> necessarily.
>>>>>>>>>> But, the bridge setlink/getlink interface was changed to
>>>>>>>>>> accommodate
>>>>>>>>
>>>>>>>> 'self'
>>>>>>>>>>
>>>>>>>>>> for exactly such cases.
>>>>>>>>>> I kind of understand your case for the other attributes (these are
>>>>>>>>>> per port settings that switch asics provide).
>>>>>>>>>>
>>>>>>>>>> However, i don't understand the reason to pull in bridge attributes
>>>>>>>>>> here.
>>>>>>>>>>
>>>>>>>>> Maybe, I am missing something so you might help. The learning
>>>>>>>>> attribute -
>>>>>>>>
>>>>>>>> in my case - it is like all other attributes: a port attribute (as
>>>>>>>> you said, port
>>>>>>>> settings that the switch provides per port).
>>>>>>>>>
>>>>>>>>> So, what I was saying is "why the user shall go through a bridge to
>>>>>>>>> configure
>>>>>>>>
>>>>>>>> the learning attribute"? From my perspective, it is as any other
>>>>>>>> attribute and
>>>>>>>> as such configurable on the port.
>>>>>>>>
>>>>>>>> Thinking about this some more, i don't see why any of these
>>>>>>>> attributes
>>>>>>>> (except loopback. I dont understand the loopback attribute) cant be
>>>>>>>> part of
>>>>>>>> the birdge port attributes.
>>>>>>>>
>>>>>>>> With this we will end up adding l2 attributes in two places: the
>>>>>>>> general link
>>>>>>>> attributes and bridge attributes.
>>>>>>>>
>>>>>>>> And since we have gone down the path of using
>>>>>>>> ndo_bridge_setlink/getlink
>>>>>>>> with 'self'....we should stick to that for all l2 attributes.
>>>>>>>>
>>>>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same
>>>>>>>> set of
>>>>>>>> attributes but support both cases where the user wants to go through
>>>>>>>> the
>>>>>>>> bridge driver or directly to the switch port driver. So, you are not
>>>>>>>> really going
>>>>>>>> through the bridge driver if you use 'self' and
>>>>>>>> ndo_bridge_setlink/getlink.
>>>>>>>>
>>>>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch
>>>>>>> was that your patch and mine were supplementary ("I think Roopa's
>>>>>>> patches are supplementary. Not all switchdev users will be backed
>>>>>>> with a Linux Bridge. I therefore welcome your patches very
>>>>>>> much")... I also understood by others that the patch made sense for
>>>>>>> the same reason. I simply do not understand why these attributes
>>>>>>> (and maybe others in the future) could not be configured directly
>>>>>>> on a standard port but have to go through a bridge.
>>>>>>>
>>>>>> ok, i am very confused in that case. The whole moving of bridge
>>>>>> attributes from the bridge driver to rtnetlink.c was to make the
>>>>>> bridge attributes accessible to any driver who wants to set l2/bridge
>>>>>> attributes on their switch ports. So, its unclear to me why we are
>>>>>> doing this parallel thing again. This move to rtnetlink.c was done
>>>>>> during the recent rocker support. so, maybe scott/jiri can elaborate
>>>>>> more.
>>>>>
>>>>>
>>>>> Not sure if this will add to the confusion or help. But you do not
>>>>> need to have the bridge.ko loaded or netdev's attached to a bridge
>>>>> to use the setlink/getlink ndo ops and netlink messages.
>>>>>
>>>>> This was intentionally done. Its already used with NIC devices to
>>>>> configure embedded bridge settings such as VEB/VEPA.
>>>>
>>>>
>>>> that helps my case, thanks.
>>>
>>>
>>> So the user interface to set/get the per-port attributes will be via
>>> 'bridge', not 'ip'
>>>
>>> bridge link set dev sw0p1 port_attr bcast_flooding 1 self
>>> bridge link get dev sw0p1 port_attr bcast_flooding self
>>
>>
>> yes, l2 attributes.
>>>
>>>
>>> We also need an interface to set per-switch attributes. Can this work?
>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>> where sw0 is a bridge representing the hardware switch.
>>
>>
>> Not today. We discussed this @ LPC, and one way to do this would be to have
>> a device
>> representing the switch asic. This is in the works.
>
>
>Can I assume that on platforms which house more than one asic (say
>two 24 port asics, interconnected via a 10G link or equivalent, to get
>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>them as a single set of ports, and not as two 'switch ports' ?
Well that really depends on particular implementation and drivers. If you
have 2 pci-e devices, I think you should expose them as 2 entities. For
sure, you can have the driver to do the masking for you. I don't believe
that is correct though.
>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH Iproute2 next] ip netns: 'ip netns id' cmd without argument should not give error
From: Vadim Kochan @ 2014-12-19 8:31 UTC (permalink / raw)
To: Mahesh Bandewar; +Cc: netdev, Stephen Hemminger, Vadim Kochan, Saied Kazemi
In-Reply-To: <1418969138-26199-1-git-send-email-maheshb@google.com>
On Thu, Dec 18, 2014 at 10:05:38PM -0800, Mahesh Bandewar wrote:
> The command 'ip netns identify' without PID parameter is supposed to
> use the self PID to identify its ns but a trivial error prevented it
> from doing so. This patch fixes that error.
>
> So before the patch -
> # ip netns id
> No pid specified
> # echo $?
> 1
> #
>
> After the patch -
> # ip netns id
> test-ns
> # echo $?
> 0
> #
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Vadim Kochan <vadim4j@gmail.com>
> Cc: Saied Kazemi <saied@google.com>
> ---
> ip/ipnetns.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ip/ipnetns.c b/ip/ipnetns.c
> index 1c8aa029073e..ec9afa2177a5 100644
> --- a/ip/ipnetns.c
> +++ b/ip/ipnetns.c
> @@ -298,7 +298,7 @@ static int netns_identify(int argc, char **argv)
> DIR *dir;
> struct dirent *entry;
>
> - if (argc < 1) {
> + if (!argc) {
> pidstr = "self";
> } else if (argc > 1) {
> fprintf(stderr, "extra arguments specified\n");
> --
> 2.2.0.rc0.207.ga3a616c
>
Hi Mahesh,
I did not get such error on the current master branch?
Did I miss something ?
Regards,
Vadim Kochan
^ permalink raw reply
* [PATCH] bonding: avoid re-entry of bond_release
From: Wengang Wang @ 2014-12-19 8:56 UTC (permalink / raw)
To: netdev; +Cc: wen.gang.wang
If bond_release is run against an interface which is already detached from
it's master, then there is an error message shown like
"<master name> cannot release <slave name>".
The call path is:
bond_do_ioctl()
bond_release()
__bond_release_one()
Though it does not really harm, the message the message is misleading.
This patch tries to avoid the message.
Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
drivers/net/bonding/bond_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 184c434..4a71bbd 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3256,7 +3256,10 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
break;
case BOND_RELEASE_OLD:
case SIOCBONDRELEASE:
- res = bond_release(bond_dev, slave_dev);
+ if (slave_dev->flags & IFF_SLAVE)
+ res = bond_release(bond_dev, slave_dev);
+ else
+ res = 0;
break;
case BOND_SETHWADDR_OLD:
case SIOCBONDSETHWADDR:
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/2] r8152: adjust r8152_submit_rx
From: Hayes Wang @ 2014-12-19 8:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
Avoid r8152_submit_rx() from submitting rx during unexpected
moment. This could reduce the time of stopping rx.
For patch #1, the tp->speed should be updated early. Then,
the patch #2 could use it to check the current linking status.
Hayes Wang (2):
r8152: adjust set_carrier
r8152: check the status before submitting rx
drivers/net/usb/r8152.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net-next 2/2] r8152: check the status before submitting rx
From: Hayes Wang @ 2014-12-19 8:56 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
In-Reply-To: <1394712342-15778-107-Taiwan-albertk-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
Don't submit the rx if the device is unplugged, linking down,
or stopped.
Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
drivers/net/usb/r8152.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 59b70c5..b39b2e4 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
{
int ret;
+ /* The rx would be stopped, so skip submitting */
+ if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
+ !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS))
+ return 0;
+
usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
agg->head, agg_buf_sz,
(usb_complete_t)read_bulk_callback, agg);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 1/2] r8152: adjust set_carrier
From: Hayes Wang @ 2014-12-19 8:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-107-Taiwan-albertk@realtek.com>
Update the tp->speed at the beginning of the function. Then,
the other fucntion could use it for checking linking status.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2d1c77e..59b70c5 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2848,26 +2848,25 @@ static void rtl8153_down(struct r8152 *tp)
static void set_carrier(struct r8152 *tp)
{
struct net_device *netdev = tp->netdev;
- u8 speed;
+ u8 old_speed = tp->speed;
clear_bit(RTL8152_LINK_CHG, &tp->flags);
- speed = rtl8152_get_speed(tp);
+ tp->speed = rtl8152_get_speed(tp);
- if (speed & LINK_STATUS) {
- if (!(tp->speed & LINK_STATUS)) {
+ if (tp->speed & LINK_STATUS) {
+ if (!(old_speed & LINK_STATUS)) {
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, &tp->flags);
netif_carrier_on(netdev);
}
} else {
- if (tp->speed & LINK_STATUS) {
+ if (old_speed & LINK_STATUS) {
netif_carrier_off(netdev);
tasklet_disable(&tp->tl);
tp->rtl_ops.disable(tp);
tasklet_enable(&tp->tl);
}
}
- tp->speed = speed;
}
static void rtl_work_func_t(struct work_struct *work)
--
2.1.0
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: B Viswanath @ 2014-12-19 9:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141219082754.GE1848@nanopsycho.orion>
On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
<snipped for ease of reading>
>>>>
>>>>
>>>> We also need an interface to set per-switch attributes. Can this work?
>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>> where sw0 is a bridge representing the hardware switch.
>>>
>>>
>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>> a device
>>> representing the switch asic. This is in the works.
>>
>>
>>Can I assume that on platforms which house more than one asic (say
>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>them as a single set of ports, and not as two 'switch ports' ?
>
> Well that really depends on particular implementation and drivers. If you
> have 2 pci-e devices, I think you should expose them as 2 entities. For
> sure, you can have the driver to do the masking for you. I don't believe
> that is correct though.
>
In a platform that houses two asic chips, IMO, the user is still
expected to manage the router as a single entity. The configuration
being applied on both asic devices need to be matching if not
identical, and may not be conflicting. The FDB is to be synchronized
so that (offloaded) switching can happen across the asics. Some of
this stuff is asic specific anyway. Another example is that of the
learning. The (hardware) learning can't be enabled on one asic, while
being disabled on another one. The general use cases I have seen are
all involving managing the 'router' as a single entity. That the
'router' is implemented with two asics instead of a single asic (with
more ports) is to be treated as an implementation detail. This is the
usual router management method that exists today.
I hope I make sense.
So I am trying to figure out what this single entity that will be used
from a user perspective. It can be a bridge, but our bridges are more
802.1q bridges. We can use the 'self' mode, but then it means that it
should reflect the entire port count, and not just an asic.
So I was trying to deduce that in our switchdevice model, the best bet
would be to leave the unification to the driver (i.e., to project the
multiple physical asics as a single virtual switch device). This
allows any 'switch' level configurations to the bridge in 'self' mode.
And then we would need to consider stacking. Stacking differs from
this multi-asic scenario since there would be multiple CPU involved.
Thanks
Vissu
>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: B Viswanath @ 2014-12-19 9:22 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+pFwJAJ71-+MZjJXPbfu5mzMp6AWNxtYR81Lx6siy3D3HfJg@mail.gmail.com>
On 19 December 2014 at 14:31, B Viswanath <marichika4@gmail.com> wrote:
> On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
> <snipped for ease of reading>
>>>>>
>>>>>
>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>> where sw0 is a bridge representing the hardware switch.
>>>>
>>>>
>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>> a device
>>>> representing the switch asic. This is in the works.
>>>
>>>
>>>Can I assume that on platforms which house more than one asic (say
>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>them as a single set of ports, and not as two 'switch ports' ?
>>
>> Well that really depends on particular implementation and drivers. If you
>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>> sure, you can have the driver to do the masking for you. I don't believe
>> that is correct though.
>>
>
> In a platform that houses two asic chips, IMO, the user is still
> expected to manage the router as a single entity. The configuration
> being applied on both asic devices need to be matching if not
> identical, and may not be conflicting. The FDB is to be synchronized
> so that (offloaded) switching can happen across the asics. Some of
> this stuff is asic specific anyway. Another example is that of the
> learning. The (hardware) learning can't be enabled on one asic, while
> being disabled on another one. The general use cases I have seen are
> all involving managing the 'router' as a single entity. That the
> 'router' is implemented with two asics instead of a single asic (with
> more ports) is to be treated as an implementation detail. This is the
> usual router management method that exists today.
>
> I hope I make sense.
>
> So I am trying to figure out what this single entity that will be used
> from a user perspective. It can be a bridge, but our bridges are more
> 802.1q bridges. We can use the 'self' mode, but then it means that it
> should reflect the entire port count, and not just an asic.
>
> So I was trying to deduce that in our switchdevice model, the best bet
> would be to leave the unification to the driver (i.e., to project the
> multiple physical asics as a single virtual switch device). This
> allows any 'switch' level configurations to the bridge in 'self' mode.
>
> And then we would need to consider stacking. Stacking differs from
> this multi-asic scenario since there would be multiple CPU involved.
>
> Thanks
> Vissu
>
Another example i can provide is that of mirroring. Imagine user
wanted to mirror all traffic from port 1 of asic 1 to port 2 of asic
2. This can be offloaded to hardware. However, user would be able to
enter such a command only if he/she can look at a single management
entity.
Thanks
Vissu
>>>
>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 9:23 UTC (permalink / raw)
To: B Viswanath
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+pFwJAJ71-+MZjJXPbfu5mzMp6AWNxtYR81Lx6siy3D3HfJg@mail.gmail.com>
Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote:
>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
><snipped for ease of reading>
>>>>>
>>>>>
>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>> where sw0 is a bridge representing the hardware switch.
>>>>
>>>>
>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>> a device
>>>> representing the switch asic. This is in the works.
>>>
>>>
>>>Can I assume that on platforms which house more than one asic (say
>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>them as a single set of ports, and not as two 'switch ports' ?
>>
>> Well that really depends on particular implementation and drivers. If you
>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>> sure, you can have the driver to do the masking for you. I don't believe
>> that is correct though.
>>
>
>In a platform that houses two asic chips, IMO, the user is still
>expected to manage the router as a single entity. The configuration
>being applied on both asic devices need to be matching if not
>identical, and may not be conflicting. The FDB is to be synchronized
>so that (offloaded) switching can happen across the asics. Some of
>this stuff is asic specific anyway. Another example is that of the
>learning. The (hardware) learning can't be enabled on one asic, while
>being disabled on another one. The general use cases I have seen are
>all involving managing the 'router' as a single entity. That the
>'router' is implemented with two asics instead of a single asic (with
>more ports) is to be treated as an implementation detail. This is the
>usual router management method that exists today.
>
>I hope I make sense.
>
>So I am trying to figure out what this single entity that will be used
>from a user perspective. It can be a bridge, but our bridges are more
>802.1q bridges. We can use the 'self' mode, but then it means that it
>should reflect the entire port count, and not just an asic.
>
>So I was trying to deduce that in our switchdevice model, the best bet
>would be to leave the unification to the driver (i.e., to project the
>multiple physical asics as a single virtual switch device). This
Is it possible to have the asic as just single one? Or is it possible to
connect asics being multiple chips maybe from multiple vendors together?
I believe that answer is "yes" in both cases. Making two separate asics
to appear as one for user is not correct in my opinion. Driver should
not do such masking. It is unclean, unextendable.
>allows any 'switch' level configurations to the bridge in 'self' mode.
>
>And then we would need to consider stacking. Stacking differs from
>this multi-asic scenario since there would be multiple CPU involved.
>
>Thanks
>Vissu
>
>>>
>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 9:35 UTC (permalink / raw)
To: B Viswanath
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+pFwLsSTo2DN=6A=Pxa3tjBnNcc9J7JTEKNnxk_-m4Ha9EMQ@mail.gmail.com>
Fri, Dec 19, 2014 at 10:22:24AM CET, marichika4@gmail.com wrote:
>On 19 December 2014 at 14:31, B Viswanath <marichika4@gmail.com> wrote:
>> On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>> <snipped for ease of reading>
>>>>>>
>>>>>>
>>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>>> where sw0 is a bridge representing the hardware switch.
>>>>>
>>>>>
>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>>> a device
>>>>> representing the switch asic. This is in the works.
>>>>
>>>>
>>>>Can I assume that on platforms which house more than one asic (say
>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>>them as a single set of ports, and not as two 'switch ports' ?
>>>
>>> Well that really depends on particular implementation and drivers. If you
>>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>>> sure, you can have the driver to do the masking for you. I don't believe
>>> that is correct though.
>>>
>>
>> In a platform that houses two asic chips, IMO, the user is still
>> expected to manage the router as a single entity. The configuration
>> being applied on both asic devices need to be matching if not
>> identical, and may not be conflicting. The FDB is to be synchronized
>> so that (offloaded) switching can happen across the asics. Some of
>> this stuff is asic specific anyway. Another example is that of the
>> learning. The (hardware) learning can't be enabled on one asic, while
>> being disabled on another one. The general use cases I have seen are
>> all involving managing the 'router' as a single entity. That the
>> 'router' is implemented with two asics instead of a single asic (with
>> more ports) is to be treated as an implementation detail. This is the
>> usual router management method that exists today.
>>
>> I hope I make sense.
>>
>> So I am trying to figure out what this single entity that will be used
>> from a user perspective. It can be a bridge, but our bridges are more
>> 802.1q bridges. We can use the 'self' mode, but then it means that it
>> should reflect the entire port count, and not just an asic.
>>
>> So I was trying to deduce that in our switchdevice model, the best bet
>> would be to leave the unification to the driver (i.e., to project the
>> multiple physical asics as a single virtual switch device). This
>> allows any 'switch' level configurations to the bridge in 'self' mode.
>>
>> And then we would need to consider stacking. Stacking differs from
>> this multi-asic scenario since there would be multiple CPU involved.
>>
>> Thanks
>> Vissu
>>
>
>Another example i can provide is that of mirroring. Imagine user
>wanted to mirror all traffic from port 1 of asic 1 to port 2 of asic
>2. This can be offloaded to hardware. However, user would be able to
>enter such a command only if he/she can look at a single management
>entity.
I understand your use case. I think this could be handled by higher
entity. In this sase tome userspace agent which would be aware (by
configuration) of all asics and how they are interconnected. Just a
thought. Seem much nice than to do custom masking in drivers.
>
>Thanks
>Vissu
>
>>>>
>>>>>
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: B Viswanath @ 2014-12-19 9:35 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141219092308.GF1848@nanopsycho.orion>
On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote:
>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>><snipped for ease of reading>
>>>>>>
>>>>>>
>>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>>> where sw0 is a bridge representing the hardware switch.
>>>>>
>>>>>
>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>>> a device
>>>>> representing the switch asic. This is in the works.
>>>>
>>>>
>>>>Can I assume that on platforms which house more than one asic (say
>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>>them as a single set of ports, and not as two 'switch ports' ?
>>>
>>> Well that really depends on particular implementation and drivers. If you
>>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>>> sure, you can have the driver to do the masking for you. I don't believe
>>> that is correct though.
>>>
>>
>>In a platform that houses two asic chips, IMO, the user is still
>>expected to manage the router as a single entity. The configuration
>>being applied on both asic devices need to be matching if not
>>identical, and may not be conflicting. The FDB is to be synchronized
>>so that (offloaded) switching can happen across the asics. Some of
>>this stuff is asic specific anyway. Another example is that of the
>>learning. The (hardware) learning can't be enabled on one asic, while
>>being disabled on another one. The general use cases I have seen are
>>all involving managing the 'router' as a single entity. That the
>>'router' is implemented with two asics instead of a single asic (with
>>more ports) is to be treated as an implementation detail. This is the
>>usual router management method that exists today.
>>
>>I hope I make sense.
>>
>>So I am trying to figure out what this single entity that will be used
>>from a user perspective. It can be a bridge, but our bridges are more
>>802.1q bridges. We can use the 'self' mode, but then it means that it
>>should reflect the entire port count, and not just an asic.
>>
>>So I was trying to deduce that in our switchdevice model, the best bet
>>would be to leave the unification to the driver (i.e., to project the
>>multiple physical asics as a single virtual switch device). Thist
>
> Is it possible to have the asic as just single one? Or is it possible to
> connect asics being multiple chips maybe from multiple vendors together?
I didn't understand the first question. Some times, it is possible to
have a single asic replace two, but its a cost factor, and others that
are involved.
AFAIK, the answer to the second question is a No. Two asics from
different vendors may not be connected together. The interconnect
tends to be proprietary.
> I believe that answer is "yes" in both cases. Making two separate asics
> to appear as one for user is not correct in my opinion. Driver should
> not do such masking. It is unclean, unextendable.
>
I am only looking for a single management entity. I am not thinking it
needs to be at driver level. I am not sure of any other option apart
from creating a 'switchdev' that Roopa was mentioning.
>
>>allows any 'switch' level configurations to the bridge in 'self' mode.
>>
>>And then we would need to consider stacking. Stacking differs from
>>this multi-asic scenario since there would be multiple CPU involved.
>>
>>Thanks
>>Vissu
>>
>>>>
>>>>>
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: Jiri Pirko @ 2014-12-19 9:55 UTC (permalink / raw)
To: B Viswanath
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+pFwJr7TvJEW4x_HYPgTEvYGfi+YHgp=5vek2YiU1GLiH6_g@mail.gmail.com>
Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote:
>On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote:
>>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>>><snipped for ease of reading>
>>>>>>>
>>>>>>>
>>>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>>>> where sw0 is a bridge representing the hardware switch.
>>>>>>
>>>>>>
>>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>>>> a device
>>>>>> representing the switch asic. This is in the works.
>>>>>
>>>>>
>>>>>Can I assume that on platforms which house more than one asic (say
>>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>>>them as a single set of ports, and not as two 'switch ports' ?
>>>>
>>>> Well that really depends on particular implementation and drivers. If you
>>>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>>>> sure, you can have the driver to do the masking for you. I don't believe
>>>> that is correct though.
>>>>
>>>
>>>In a platform that houses two asic chips, IMO, the user is still
>>>expected to manage the router as a single entity. The configuration
>>>being applied on both asic devices need to be matching if not
>>>identical, and may not be conflicting. The FDB is to be synchronized
>>>so that (offloaded) switching can happen across the asics. Some of
>>>this stuff is asic specific anyway. Another example is that of the
>>>learning. The (hardware) learning can't be enabled on one asic, while
>>>being disabled on another one. The general use cases I have seen are
>>>all involving managing the 'router' as a single entity. That the
>>>'router' is implemented with two asics instead of a single asic (with
>>>more ports) is to be treated as an implementation detail. This is the
>>>usual router management method that exists today.
>>>
>>>I hope I make sense.
>>>
>>>So I am trying to figure out what this single entity that will be used
>>>from a user perspective. It can be a bridge, but our bridges are more
>>>802.1q bridges. We can use the 'self' mode, but then it means that it
>>>should reflect the entire port count, and not just an asic.
>>>
>>>So I was trying to deduce that in our switchdevice model, the best bet
>>>would be to leave the unification to the driver (i.e., to project the
>>>multiple physical asics as a single virtual switch device). Thist
>>
>> Is it possible to have the asic as just single one? Or is it possible to
>> connect asics being multiple chips maybe from multiple vendors together?
>
>I didn't understand the first question. Some times, it is possible to
I ment that there is a design with just a single asic of this type,
instead of a pair.
>have a single asic replace two, but its a cost factor, and others that
>are involved.
>
>AFAIK, the answer to the second question is a No. Two asics from
>different vendors may not be connected together. The interconnect
>tends to be proprietary.
Okay. In that case, it might make sense to mask it on driver level.
>
>> I believe that answer is "yes" in both cases. Making two separate asics
>> to appear as one for user is not correct in my opinion. Driver should
>> not do such masking. It is unclean, unextendable.
>>
>
>I am only looking for a single management entity. I am not thinking it
>needs to be at driver level. I am not sure of any other option apart
>from creating a 'switchdev' that Roopa was mentioning.
Well the thing is there is a common desire to make the offloading as
transparent as possible. For example, have 4 ports of same switch and
put them into br0. Just like that, without need to do anything else
than you would do when bridging ordinary NICs. Introducing some
"management entity" would break this approach.
>
>>
>>>allows any 'switch' level configurations to the bridge in 'self' mode.
>>>
>>>And then we would need to consider stacking. Stacking differs from
>>>this multi-asic scenario since there would be multiple CPU involved.
>>>
>>>Thanks
>>>Vissu
>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
From: Jason Wang @ 2014-12-19 10:02 UTC (permalink / raw)
To: Qin Chuanyu; +Cc: pagupta, mst, netdev, linux-kernel, virtualization, davem
In-Reply-To: <5493D488.7020000@huawei.com>
On Fri, Dec 19, 2014 at 3:32 PM, Qin Chuanyu <qinchuanyu@huawei.com>
wrote:
> On 2014/12/1 18:17, Jason Wang wrote:
>> On newer hosts that support delayed tx interrupts,
>> we probably don't have much to gain from orphaning
>> packets early.
>>
>> Note: this might degrade performance for
>> hosts without event idx support.
>> Should be addressed by the next patch.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/net/virtio_net.c | 132
>> +++++++++++++++++++++++++++++++----------------
>> 1 file changed, 88 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index ec2a8b4..f68114e 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
>> {
>> struct skb_vnet_hdr *hdr;
>> @@ -912,7 +951,9 @@ static int xmit_skb(struct send_queue *sq,
>> struct sk_buff *skb)
>> sg_set_buf(sq->sg, hdr, hdr_len);
>> num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
>> }
>> - return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
>> GFP_ATOMIC);
>> +
>> + return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
>> + GFP_ATOMIC);
>> }
>>
>> static netdev_tx_t start_xmit(struct sk_buff *skb, struct
>> net_device *dev)
>> @@ -924,8 +965,7 @@ static netdev_tx_t start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>> struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
>> bool kick = !skb->xmit_more;
>>
>> - /* Free up any pending old buffers before queueing new ones. */
>> - free_old_xmit_skbs(sq);
>
> I think there is no need to remove free_old_xmit_skbs here.
> you could add free_old_xmit_skbs in tx_irq's napi func.
> also could do this in start_xmit if you handle the race well.
Note, free_old_xmit_skbs() has already called in tx napi.
It was a must after tx interrupt was enabled.
>
>
> I have done the same thing in ixgbe driver(free skb in ndo_start_xmit
> and both in tx_irq's poll func), and it seems work well:)
Any performance numbers on this change?
I suspect it reduce the effects of interrupt coalescing.
>
> I think there would be no so much interrupts in this way, also tx
> interrupt coalesce is not needed.
Tests (multiple sessions of TCP_RR) does not support this.
Calling free_old_xmit_skbs() in fact damage the performance.
Any justification that you think it may reduce the interrupts?
Thanks
>
>
>> + virtqueue_disable_cb(sq->vq);
>>
>> /* Try to transmit */
>> err = xmit_skb(sq, skb);
>> @@ -941,27 +981,19 @@ static netdev_tx_t start_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>> return NETDEV_TX_OK;
>> }
>>
>> - /* Don't wait up for transmitted skbs to be freed. */
>> - skb_orphan(skb);
>> - nf_reset(skb);
>> -
>> /* Apparently nice girls don't return TX_BUSY; stop the queue
>> * before it gets out of hand. Naturally, this wastes entries. */
>> - if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
>> + if (sq->vq->num_free < 2+MAX_SKB_FRAGS)
>> netif_stop_subqueue(dev, qnum);
>> - if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>> - /* More just got used, free them then recheck. */
>> - free_old_xmit_skbs(sq);
>> - if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
>> - netif_start_subqueue(dev, qnum);
>> - virtqueue_disable_cb(sq->vq);
>> - }
>> - }
>> - }
>>
>> if (kick || netif_xmit_stopped(txq))
>> virtqueue_kick(sq->vq);
>>
>> + if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
>> + virtqueue_disable_cb(sq->vq);
>> + napi_schedule(&sq->napi);
>> + }
>> +
>> return NETDEV_TX_OK;
>> }
>>
>> @@ -1138,8 +1170,10 @@ static int virtnet_close(struct net_device
>> *dev)
>> /* Make sure refill_work doesn't re-enable napi! */
>> cancel_delayed_work_sync(&vi->refill);
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++)
>> + for (i = 0; i < vi->max_queue_pairs; i++) {
>> napi_disable(&vi->rq[i].napi);
>> + napi_disable(&vi->sq[i].napi);
>> + }
>>
>> return 0;
>> }
>> @@ -1452,8 +1486,10 @@ static void virtnet_free_queues(struct
>> virtnet_info *vi)
>> {
>> int i;
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++)
>> + for (i = 0; i < vi->max_queue_pairs; i++) {
>> netif_napi_del(&vi->rq[i].napi);
>> + netif_napi_del(&vi->sq[i].napi);
>> + }
>>
>> kfree(vi->rq);
>> kfree(vi->sq);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] enic: fix rx skb checksum
From: Jiri Benc @ 2014-12-19 10:07 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Eric Dumazet, Govindarajulu Varadarajan, davem, netdev, ssujith,
benve, Stefan Assmann
In-Reply-To: <17951.1418924367@famine>
On Thu, 18 Dec 2014 09:39:27 -0800, Jay Vosburgh wrote:
> I've actually been looking into this "hw csum failure" (as it
> appears with OVS and VXLAN) the last couple of days, and, at least on
> the sky2 hardware I have, the problem doesn't appear to be the
> hardware's CHECKSUM_COMPLETE checksumming.
With the enic driver, the problem _is_ the hardware checksumming.
While debugging the "hw csum failure" messages, I verified the checksum
returned by the hardware directly in the driver (in
enic_rq_indicate_buf). It appears that for some packets (most notably
ICMP ones), the hardware returns 0xffff. I did not see any other wrong
value, in other words, the returned checksum was either correct, or
0xffff.
I have no idea whether the hardware verified the checksum for the
0xffff case and is just not returning the checksum or whether such
packets come completely unverified.
As for Govindarajulu's patch, I believe we can do better. First, its
description does not match what I'm seeing (I see correct checksum
provided in most cases) and second, the driver should provide
CHECKSUM_COMPLETE whenever possible; and it seems to be possible.
> I've also not tested it on enic hardware. Govindarajulu, would
> you be able to test this against the unmodified enic driver and see if
> it resolves the problem for you?
I'm quite sure it does not, the skb->csum field is incorrect even
before the skb enters the stack.
Jiri
--
Jiri Benc
^ permalink raw reply
* Re: [patches] a bunch of old bluetooth fixes
From: Marcel Holtmann @ 2014-12-19 10:28 UTC (permalink / raw)
To: Al Viro; +Cc: David S. Miller, netdev, linux-bluetooth
In-Reply-To: <20141219061801.GU22149@ZenIV.linux.org.uk>
Hi Al,
> This stuff has been sitting in my queue since March; basically,
> several places in net/bluetooth assume that they are dealing with
> l2cap sockets, while it is possible to get an arbitrary socket to those.
> Results are not pretty.
> * HIDPCONNADD gets an arbitrary user-supplied socket; the code
> it calls (hidp_connection_add()) verifies that the socket is l2cap one,
> but before doing so it finds l2cap_pi(ctrl_sock->sk)->chan. It's not
> that big a deal (it's only 5 words past the end of struct sock), but
> it's trivial to avoid and, in theory, we might end up oopsing here if
> we are very unlucky and it happens to hit an unmapped page just past
> the actual object ctrl_sock->sk sits in.
> * CMTP counterpart of that doesn't validate the socket at all.
> It proceeds to
> s = __cmtp_get_session(&l2cap_pi(sock->sk)->chan->dst);
> which can very easily oops - here ->chan is already garbage and we
> proceed to dereference that. As with HIDP, one needs CAP_NET_ADMIN to
> trigger that, but it's really a clear bug. The only sanity check we
> do is verifying that nsock->sk->sk_state is equal to BT_CONNECTED,
> which is not unique to bluetooth, to put it mildly. It's just 1,
> so a TCP_ESTABLISHED tcp socket will pass that check just fune.
> The fix is trivial...
> * BNEP situation is identical to CMTP one.
>
> I've sent these patches back then (March 10), but they seem to have fallen
> through the cracks. The bugs are still there and the fixes still apply.
> If you would prefer me to resend them after -rc1, just tell...
they must have really fallen through the cracks since I do not even remember them.
My take is that these should all go in before -rc1 and preferable also make it into stable. While you need CAP_NET_ADMIN capability, there are clear stupid bugs on our side.
Dave, we can prepare a pull request for these or do you want to take them directly into net tree?
Regards
Marcel
^ permalink raw reply
* Re: [patches] a bunch of old bluetooth fixes
From: Marcel Holtmann @ 2014-12-19 10:30 UTC (permalink / raw)
To: Al Viro, David S. Miller; +Cc: Network Development, BlueZ development
In-Reply-To: <B0609FEE-1CE5-4618-A0B6-B2B82B1EC74D@holtmann.org>
Hi Dave,
>> This stuff has been sitting in my queue since March; basically,
>> several places in net/bluetooth assume that they are dealing with
>> l2cap sockets, while it is possible to get an arbitrary socket to those.
>> Results are not pretty.
>> * HIDPCONNADD gets an arbitrary user-supplied socket; the code
>> it calls (hidp_connection_add()) verifies that the socket is l2cap one,
>> but before doing so it finds l2cap_pi(ctrl_sock->sk)->chan. It's not
>> that big a deal (it's only 5 words past the end of struct sock), but
>> it's trivial to avoid and, in theory, we might end up oopsing here if
>> we are very unlucky and it happens to hit an unmapped page just past
>> the actual object ctrl_sock->sk sits in.
>> * CMTP counterpart of that doesn't validate the socket at all.
>> It proceeds to
>> s = __cmtp_get_session(&l2cap_pi(sock->sk)->chan->dst);
>> which can very easily oops - here ->chan is already garbage and we
>> proceed to dereference that. As with HIDP, one needs CAP_NET_ADMIN to
>> trigger that, but it's really a clear bug. The only sanity check we
>> do is verifying that nsock->sk->sk_state is equal to BT_CONNECTED,
>> which is not unique to bluetooth, to put it mildly. It's just 1,
>> so a TCP_ESTABLISHED tcp socket will pass that check just fune.
>> The fix is trivial...
>> * BNEP situation is identical to CMTP one.
>>
>> I've sent these patches back then (March 10), but they seem to have fallen
>> through the cracks. The bugs are still there and the fixes still apply.
>> If you would prefer me to resend them after -rc1, just tell...
>
> they must have really fallen through the cracks since I do not even remember them.
>
> My take is that these should all go in before -rc1 and preferable also make it into stable. While you need CAP_NET_ADMIN capability, there are clear stupid bugs on our side.
>
> Dave, we can prepare a pull request for these or do you want to take them directly into net tree?
and in case you decide to take them directly.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net] enic: fix rx skb checksum
From: Govindarajulu Varadarajan @ 2014-12-19 10:52 UTC (permalink / raw)
To: Jiri Benc
Cc: Jay Vosburgh, Eric Dumazet, Govindarajulu Varadarajan, davem,
netdev, ssujith, benve, Stefan Assmann
In-Reply-To: <20141219110732.53264ef8@griffin>
On Fri, 19 Dec 2014, Jiri Benc wrote:
> On Thu, 18 Dec 2014 09:39:27 -0800, Jay Vosburgh wrote:
>> I've actually been looking into this "hw csum failure" (as it
>> appears with OVS and VXLAN) the last couple of days, and, at least on
>> the sky2 hardware I have, the problem doesn't appear to be the
>> hardware's CHECKSUM_COMPLETE checksumming.
>
> With the enic driver, the problem _is_ the hardware checksumming.
>
> While debugging the "hw csum failure" messages, I verified the checksum
> returned by the hardware directly in the driver (in
> enic_rq_indicate_buf). It appears that for some packets (most notably
> ICMP ones), the hardware returns 0xffff. I did not see any other wrong
> value, in other words, the returned checksum was either correct, or
> 0xffff.
>
Hardware returns 0xffff for non tcp/udp packets. For tcp/udp packet it returns
pseudo checksum. Not the _whole_ pkt checksum.
With the following changes, I see this output:
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index e3dc629..0f2be67 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1092,6 +1092,24 @@ static void enic_rq_indicate_buf(struct vnic_rq *rq,
}
if ((netdev->features & NETIF_F_RXCSUM) && !csum_not_calc) {
+ if (printk_ratelimit()) {
+ const struct iphdr *iph = (struct iphdr *)skb->data;
+ __u16 length_for_csum = 0;
+ __wsum pseudo_csum = 0;
+
+ length_for_csum = (be16_to_cpu(iph->tot_len) - (iph->ihl << 2));
+ pseudo_csum = csum_tcpudp_nofold(iph->saddr,
+ iph->daddr,
+ length_for_csum,
+ iph->protocol, 0);
+
+ pr_info("saddr=%x, daddr=%x, length=%d, proto=%d\n",
+ iph->saddr, iph->daddr, length_for_csum, iph->protocol);
+ pr_info("hw_checksum = %x, pseudo_checksum_32=%x, pseudo_checksum_fold=%x\n",
+ htons(checksum),
+ pseudo_csum,
+ csum_fold(pseudo_csum));
+ }
skb->csum = htons(checksum);
skb->ip_summed = CHECKSUM_COMPLETE;
}
Output:
Dec 18 11:13:18 a163 kernel: enic: saddr=96d8690a, daddr=a3ba6a0a, length=40, proto=6
Dec 18 11:13:18 a163 kernel: enic: hw_checksum = c457, pseudo_checksum_32=3a930115, pseudo_checksum_fold=c457
Dec 18 11:13:18 a163 kernel: enic: saddr=a37410a, daddr=a3ba6a0a, length=32, proto=6
Dec 18 11:13:18 a163 kernel: enic: hw_checksum = 80f9, pseudo_checksum_32=adf1d114, pseudo_checksum_fold=80f9
Dec 18 11:13:18 a163 kernel: enic: saddr=a37410a, daddr=a3ba6a0a, length=32, proto=6
Dec 18 11:13:18 a163 kernel: enic: hw_checksum = 80f9, pseudo_checksum_32=adf1d114, pseudo_checksum_fold=80f9
Clearly hw is returning folded pseudo checksum.
> I have no idea whether the hardware verified the checksum for the
> 0xffff case and is just not returning the checksum or whether such
> packets come completely unverified.
>
Yes, hardware verifies the checksum and sets tcp_udp_csum_ok flag to 1.
If pkt verification fails or pkt is not tcp/udp, tcp_udp_csum_ok is 0. And we
send the pkt to stack with CHECKSUM_NONE.
> As for Govindarajulu's patch, I believe we can do better. First, its
> description does not match what I'm seeing (I see correct checksum
> provided in most cases) and second, the driver should provide
> CHECKSUM_COMPLETE whenever possible; and it seems to be possible.
>
Driver should use CHECKSUM_COMPLETE only if it can produce _whole_ pkt checksum.
as described in include/linux/skbuff.h:75
* CHECKSUM_COMPLETE:
*
* This is the most generic way. The device supplied checksum of the _whole_
* packet as seen by netif_rx() and fills out in skb->csum. Meaning, the
* hardware doesn't need to parse L3/L4 headers to implement this.
*
* Note: Even if device supports only some protocols, but is able to produce
* skb->csum, it MUST use CHECKSUM_COMPLETE, not CHECKSUM_UNNECESSARY.
Since enic hw verifies checksum but does not provide us whole pkt checksum,
it should use CHECKSUM_UNNECESSARY. as described in include/linux/skbuff.h:47
* CHECKSUM_UNNECESSARY:
*
* The hardware you're dealing with doesn't calculate the full checksum
* (as in CHECKSUM_COMPLETE), but it does parse headers and verify checksums
* for specific protocols. For such packets it will set CHECKSUM_UNNECESSARY
* if their checksums are okay. skb->csum is still undefined in this case
Am I correct?
>> I've also not tested it on enic hardware. Govindarajulu, would
>> you be able to test this against the unmodified enic driver and see if
>> it resolves the problem for you?
>
I think Jay Vosburgh's patch and my patch are addressing two different issues.
I am trying to fix "Do not set CHECKSUM_COMPLETE, when driver does not have
checksum of whole pkt."
Is my understanding correct?
Thanks
> I'm quite sure it does not, the skb->csum field is incorrect even
> before the skb enters the stack.
>
> Jiri
>
> --
> Jiri Benc
>
^ permalink raw reply related
* Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: B Viswanath @ 2014-12-19 10:53 UTC (permalink / raw)
To: Jiri Pirko
Cc: Roopa Prabhu, Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, sfeldma@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141219095512.GH1848@nanopsycho.orion>
On 19 December 2014 at 15:25, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Dec 19, 2014 at 10:35:27AM CET, marichika4@gmail.com wrote:
>>On 19 December 2014 at 14:53, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Fri, Dec 19, 2014 at 10:01:46AM CET, marichika4@gmail.com wrote:
>>>>On 19 December 2014 at 13:57, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Fri, Dec 19, 2014 at 06:14:57AM CET, marichika4@gmail.com wrote:
>>>>>>On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>>>>> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>>>><snipped for ease of reading>
>>>>>>>>
>>>>>>>>
>>>>>>>> We also need an interface to set per-switch attributes. Can this work?
>>>>>>>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>>>>>>>> where sw0 is a bridge representing the hardware switch.
>>>>>>>
>>>>>>>
>>>>>>> Not today. We discussed this @ LPC, and one way to do this would be to have
>>>>>>> a device
>>>>>>> representing the switch asic. This is in the works.
>>>>>>
>>>>>>
>>>>>>Can I assume that on platforms which house more than one asic (say
>>>>>>two 24 port asics, interconnected via a 10G link or equivalent, to get
>>>>>>a 48 port 'switch') , the 'rocker' driver (or similar) should expose
>>>>>>them as a single set of ports, and not as two 'switch ports' ?
>>>>>
>>>>> Well that really depends on particular implementation and drivers. If you
>>>>> have 2 pci-e devices, I think you should expose them as 2 entities. For
>>>>> sure, you can have the driver to do the masking for you. I don't believe
>>>>> that is correct though.
>>>>>
>>>>
>>>>In a platform that houses two asic chips, IMO, the user is still
>>>>expected to manage the router as a single entity. The configuration
>>>>being applied on both asic devices need to be matching if not
>>>>identical, and may not be conflicting. The FDB is to be synchronized
>>>>so that (offloaded) switching can happen across the asics. Some of
>>>>this stuff is asic specific anyway. Another example is that of the
>>>>learning. The (hardware) learning can't be enabled on one asic, while
>>>>being disabled on another one. The general use cases I have seen are
>>>>all involving managing the 'router' as a single entity. That the
>>>>'router' is implemented with two asics instead of a single asic (with
>>>>more ports) is to be treated as an implementation detail. This is the
>>>>usual router management method that exists today.
>>>>
>>>>I hope I make sense.
>>>>
>>>>So I am trying to figure out what this single entity that will be used
>>>>from a user perspective. It can be a bridge, but our bridges are more
>>>>802.1q bridges. We can use the 'self' mode, but then it means that it
>>>>should reflect the entire port count, and not just an asic.
>>>>
>>>>So I was trying to deduce that in our switchdevice model, the best bet
>>>>would be to leave the unification to the driver (i.e., to project the
>>>>multiple physical asics as a single virtual switch device). Thist
>>>
>>> Is it possible to have the asic as just single one? Or is it possible to
>>> connect asics being multiple chips maybe from multiple vendors together?
>>
>>I didn't understand the first question. Some times, it is possible to
>
> I ment that there is a design with just a single asic of this type,
> instead of a pair.
>
>>have a single asic replace two, but its a cost factor, and others that
>>are involved.
>>
>>AFAIK, the answer to the second question is a No. Two asics from
>>different vendors may not be connected together. The interconnect
>>tends to be proprietary.
>
> Okay. In that case, it might make sense to mask it on driver level.
>
>
>>
>>> I believe that answer is "yes" in both cases. Making two separate asics
>>> to appear as one for user is not correct in my opinion. Driver should
>>> not do such masking. It is unclean, unextendable.
>>>
>>
>>I am only looking for a single management entity. I am not thinking it
>>needs to be at driver level. I am not sure of any other option apart
>>from creating a 'switchdev' that Roopa was mentioning.
>
> Well the thing is there is a common desire to make the offloading as
> transparent as possible. For example, have 4 ports of same switch and
> put them into br0. Just like that, without need to do anything else
> than you would do when bridging ordinary NICs. Introducing some
> "management entity" would break this approach.
>
This is a simple and solid approach that works fine as long as the
asics can be viewed as collection of ports. This is true for many
usecases. However, the asics are more than a collection of ports,
having shared and common infrastructure among the ports. They offer
configuration options that are not specific to any port, but
'belonging' to the asic. So there may be a need in some usecases which
involve in setting up these config options, to somehow identify that
these belong to the asic itself. If there is a single asic, we can
have implied identification, but not with multiple asics.
I guess if we can manage multiple asics belonging to same vendor
within the driver, we probably don't need any such identification (and
that was my original question). We already have the 'self' bridge
which can be enhanced for any properties. However, at this point I am
not sure how the routing plays our with the 'self' bridge. IMHO, it
would be neat to have a 'switchdev' which can cleanly handle such
stuff.
>>
>>>
>>>>allows any 'switch' level configurations to the bridge in 'self' mode.
>>>>
>>>>And then we would need to consider stacking. Stacking differs from
>>>>this multi-asic scenario since there would be multiple CPU involved.
>>>>
>>>>Thanks
>>>>Vissu
>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] enic: fix rx skb checksum
From: Jiri Benc @ 2014-12-19 11:07 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: Jay Vosburgh, Eric Dumazet, davem, netdev, ssujith, benve,
Stefan Assmann
In-Reply-To: <alpine.LNX.2.03.1412191548001.2860@ws.cisco>
On Fri, 19 Dec 2014 16:22:34 +0530 (IST), Govindarajulu Varadarajan wrote:
> Hardware returns 0xffff for non tcp/udp packets.
That explains that I saw that with ICMP packets.
> For tcp/udp packet it returns
> pseudo checksum. Not the _whole_ pkt checksum.
I see. I didn't get this from your patch, although you wrote it there,
sorry. And I didn't dig that deep while debugging, I was satisfied with
seeing 0xffff for the packets that caused problems :-)
> Dec 18 11:13:18 a163 kernel: enic: saddr=96d8690a, daddr=a3ba6a0a, length=40, proto=6
> Dec 18 11:13:18 a163 kernel: enic: hw_checksum = c457, pseudo_checksum_32=3a930115, pseudo_checksum_fold=c457
>
> Dec 18 11:13:18 a163 kernel: enic: saddr=a37410a, daddr=a3ba6a0a, length=32, proto=6
> Dec 18 11:13:18 a163 kernel: enic: hw_checksum = 80f9, pseudo_checksum_32=adf1d114, pseudo_checksum_fold=80f9
>
> Dec 18 11:13:18 a163 kernel: enic: saddr=a37410a, daddr=a3ba6a0a, length=32, proto=6
> Dec 18 11:13:18 a163 kernel: enic: hw_checksum = 80f9, pseudo_checksum_32=adf1d114, pseudo_checksum_fold=80f9
>
> Clearly hw is returning folded pseudo checksum.
Indeed.
> > I have no idea whether the hardware verified the checksum for the
> > 0xffff case and is just not returning the checksum or whether such
> > packets come completely unverified.
>
> Yes, hardware verifies the checksum and sets tcp_udp_csum_ok flag to 1.
> If pkt verification fails or pkt is not tcp/udp, tcp_udp_csum_ok is 0. And we
> send the pkt to stack with CHECKSUM_NONE.
Thanks for the confirmation.
> Driver should use CHECKSUM_COMPLETE only if it can produce _whole_ pkt checksum.
> as described in include/linux/skbuff.h:75
Sure. I just misunderstood what the hardware provides.
> I am trying to fix "Do not set CHECKSUM_COMPLETE, when driver does not have
> checksum of whole pkt."
>
> Is my understanding correct?
With the information you've just written (thanks for your patience),
I think your patch is correct.
Thanks a lot!
Jiri
--
Jiri Benc
^ permalink raw reply
* Re: [PATCH net] enic: fix rx skb checksum
From: Jiri Benc @ 2014-12-19 11:11 UTC (permalink / raw)
To: Govindarajulu Varadarajan; +Cc: davem, netdev, ssujith, benve, Stefan Assmann
In-Reply-To: <1418898522-13588-1-git-send-email-_govind@gmx.com>
On Thu, 18 Dec 2014 15:58:42 +0530, Govindarajulu Varadarajan wrote:
> Hardware always provides compliment of IP pseudo checksum. Stack expects
> whole packet checksum without pseudo checksum if CHECKSUM_COMPLETE is set.
>
> This causes checksum error in nf & ovs.
>
> [...]
>
> Hardware verifies IP & tcp/udp header checksum but does not provide payload
> checksum, use CHECKSUM_UNNECESSARY. Set it only if its valid IP tcp/udp packet.
>
> Cc: Jiri Benc <jbenc@redhat.com>
> Cc: Stefan Assmann <sassmann@redhat.com>
> Reported-by: Sunil Choudhary <schoudha@redhat.com>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
Reviewed-by: Jiri Benc <jbenc@redhat.com>
--
Jiri Benc
^ permalink raw reply
* Recent Linus' tree, kernel BUG at fs/inode.c:1436!
From: Pavel Emelyanov @ 2014-12-19 11:34 UTC (permalink / raw)
To: Linux Netdev List, linux-fsdevel
Hi,
It looks like there's a strange refcount underflow in VFS/socket code.
The proggie [1] crashes the recent Linus' tree (d790be38 Merge tag
'modules-next-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux)
with the calltrace [2].
If in the proggie the psk is replaced with non-socket descriptor the
issue doesn't appear.
Thanks,
Pavel
[1]--------------------------------------------
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <stdio.h>
#include <fcntl.h>
int main(int argc, char **argv)
{
int psk, proc;
char buf[1024];
psk = socket(PF_LOCAL, SOCK_STREAM, 0);
sprintf(buf, "/proc/self/fd/%d", psk);
proc = open(buf, O_RDONLY);
close(proc);
close(psk);
return 0;
}
[2]--------------------------------------------
[ 322.823998] ------------[ cut here ]------------
[ 322.824251] kernel BUG at fs/inode.c:1436!
[ 322.824251] invalid opcode: 0000 [#6] SMP
[ 322.824251] Modules linked in: serio_raw ata_generic
[ 322.824251] CPU: 0 PID: 1058 Comm: a.out Tainted: G D 3.18.0-criu+ #10
[ 322.824251] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 322.824251] task: ffff88003ca7d070 ti: ffff880037058000 task.ti: ffff880037058000
[ 322.824251] RIP: 0010:[<ffffffff811521bf>] [<ffffffff811521bf>] iput+0x13f/0x180
[ 322.824251] RSP: 0018:ffff88003705be08 EFLAGS: 00010202
[ 322.824251] RAX: 0000000000000000 RBX: ffff88003dd0dbb0 RCX: dead000000200200
[ 322.824251] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88003dd0dbb0
[ 322.824251] RBP: ffff88003705be28 R08: ffff88003d187a50 R09: ffff88003fc15820
[ 322.824251] R10: ffffffff81392d4e R11: ffffea0000f74300 R12: ffff88003dd0dbb0
[ 322.824251] R13: ffff88003d187a18 R14: 0000000000000000 R15: ffff88003d1879c0
[ 322.824251] FS: 00007f1720ba7740(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 322.824251] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 322.824251] CR2: 00007f17206c4630 CR3: 000000003731d000 CR4: 00000000000006f0
[ 322.824251] Stack:
[ 322.824251] ffff88003d1879c0 ffff88003dd0dbb0 ffff88003d187a18 0000000000000000
[ 322.824251] ffff88003705be58 ffffffff8114d710 ffff88003705be58 0000000000000000
[ 322.824251] ffff88003d187a18 ffff88003dd0dbb0 ffff88003705be88 ffffffff8114d7fd
[ 322.824251] Call Trace:
[ 322.824251] [<ffffffff8114d710>] __dentry_kill+0x180/0x1d0
[ 322.824251] [<ffffffff8114d7fd>] dput+0x9d/0x160
[ 322.824251] [<ffffffff81139198>] __fput+0x168/0x1e0
[ 322.824251] [<ffffffff81139259>] ____fput+0x9/0x10
[ 322.824251] [<ffffffff81065d2f>] task_work_run+0xaf/0xf0
[ 322.824251] [<ffffffff81002c71>] do_notify_resume+0x51/0x80
[ 322.824251] [<ffffffff81494920>] int_signal+0x12/0x17
[ 322.824251] Code: 00 00 00 eb a0 0f 1f 80 00 00 00 00 48 81 8b 90 00 00 00 00 01 00 00 48 89 df e8 7d fe ff ff 80 83 80 00 00 00 01 e9 fb fe ff ff <0f> 0b be 71 05 00 00 48 c7 c7 53 2a 75 81 e8 5e a9 ef ff e9 06
[ 322.824251] RIP [<ffffffff811521bf>] iput+0x13f/0x180
[ 322.824251] RSP <ffff88003705be08>
[ 322.855968] ---[ end trace 2e35ddcef73fed30 ]---
^ permalink raw reply
* [RFC iproute2] tc: Show classes in tree view
From: Vadim Kochan @ 2014-12-19 11:43 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
From: Vadim Kochan <vadim4j@gmail.com>
Added new '-t[ree]' which shows classes dependency
in the tree view.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
include/hlist.h | 6 ++
tc/tc.c | 5 +-
tc/tc_class.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
tc/tc_common.h | 2 +
4 files changed, 180 insertions(+), 3 deletions(-)
diff --git a/include/hlist.h b/include/hlist.h
index 4e8de9e..dd3e606 100644
--- a/include/hlist.h
+++ b/include/hlist.h
@@ -53,4 +53,10 @@ static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h)
pos; \
pos = hlist_entry_safe((pos)->member.next, typeof(*(pos)), member))
+static inline void INIT_HLIST_NODE(struct hlist_node *h)
+{
+ h->next = NULL;
+ h->pprev = NULL;
+}
+
#endif /* __HLIST_H__ */
diff --git a/tc/tc.c b/tc/tc.c
index 9b50e74..a7986c1 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -34,8 +34,9 @@ int show_stats = 0;
int show_details = 0;
int show_raw = 0;
int show_pretty = 0;
-int batch_mode = 0;
+int show_tree = 0;
+int batch_mode = 0;
int resolve_hosts = 0;
int use_iec = 0;
int force = 0;
@@ -278,6 +279,8 @@ int main(int argc, char **argv)
++show_raw;
} else if (matches(argv[1], "-pretty") == 0) {
++show_pretty;
+ } else if (matches(argv[1], "-tree") == 0) {
+ ++show_tree;
} else if (matches(argv[1], "-Version") == 0) {
printf("tc utility, iproute2-ss%s\n", SNAPSHOT);
return 0;
diff --git a/tc/tc_class.c b/tc/tc_class.c
index e56bf07..a14944c 100644
--- a/tc/tc_class.c
+++ b/tc/tc_class.c
@@ -24,6 +24,22 @@
#include "utils.h"
#include "tc_util.h"
#include "tc_common.h"
+#include "hlist.h"
+
+struct cls_node {
+ struct hlist_node hlist;
+ __u32 handle;
+ __u32 parent;
+ int level;
+ struct cls_node *cls_parent;
+ struct cls_node *cls_right;
+ struct rtattr *attr;
+ int attr_len;
+ int childs_count;
+};
+
+static struct hlist_head cls_list = {};
+static struct hlist_head root_cls_list = {};
static void usage(void);
@@ -148,13 +164,152 @@ int filter_ifindex;
__u32 filter_qdisc;
__u32 filter_classid;
+static void tree_cls_add(__u32 parent, __u32 handle, struct rtattr *attr, int len)
+{
+ struct cls_node *cls = malloc(sizeof(struct cls_node));
+
+ memset(cls, 0, sizeof(*cls));
+ cls->handle = handle;
+ cls->parent = parent;
+ cls->attr = malloc(len);
+ cls->attr_len = len;
+
+ memcpy(cls->attr, attr, len);
+
+ if (parent == TC_H_ROOT)
+ hlist_add_head(&cls->hlist, &root_cls_list);
+ else
+ hlist_add_head(&cls->hlist, &cls_list);
+}
+
+static void tree_cls_indent(char *buf, struct cls_node *cls, int new_line)
+{
+ while (cls && cls->cls_parent) {
+ cls->cls_parent->cls_right = cls;
+ cls = cls->cls_parent;
+ }
+ while (cls && cls->cls_right)
+ {
+ if (cls->hlist.next)
+ strcat(buf, "| ");
+ else
+ strcat(buf, " ");
+
+ cls = cls->cls_right;
+ }
+
+ if (new_line) {
+ if (cls->hlist.next && cls->childs_count)
+ strcat(buf, "| |");
+ else if (cls->hlist.next)
+ strcat(buf, "| ");
+ else if (cls->childs_count)
+ strcat(buf, " |");
+ else if (!cls->hlist.next)
+ strcat(buf, " ");
+ }
+}
+
+static void tree_cls_newline(char *buf, struct cls_node *cls, int indent)
+{
+ char str[100] = {};
+
+ tree_cls_indent(buf, cls, 1);
+ sprintf(str, "%-*s", indent, "");
+ strcat(buf, str);
+}
+
+static void tree_cls_show(FILE *fp, char *buf, struct hlist_head *root_list, int level)
+{
+ struct hlist_node *n, *tmp_cls;
+ char cls_id_str[256] = {};
+ struct rtattr * tb[TCA_MAX+1] = {};
+ struct qdisc_util *q;
+ char str[100] = {};
+
+ hlist_for_each_safe(n, tmp_cls, root_list) {
+ struct hlist_node *c, *tmp_chld;
+ struct hlist_head childs = {};
+ struct cls_node *cls = container_of(n, struct cls_node, hlist);
+
+ hlist_for_each_safe(c, tmp_chld, &cls_list) {
+ struct cls_node *child = container_of(c, struct cls_node, hlist);
+
+ if (cls->handle == child->parent) {
+ hlist_del(c);
+ INIT_HLIST_NODE(c);
+ hlist_add_head(c, &childs);
+ cls->childs_count++;
+ child->cls_parent = cls;
+ }
+ }
+
+ tree_cls_indent(buf, cls, 0);
+
+ print_tc_classid(cls_id_str, sizeof(cls_id_str), cls->handle);
+ sprintf(str, "+---%s", cls_id_str);
+ strcat(buf, str);
+
+ parse_rtattr(tb, TCA_MAX, cls->attr, cls->attr_len);
+
+ if (tb[TCA_KIND] == NULL) {
+ strcat(buf, "(none)");
+ } else {
+ q = get_qdisc_kind(rta_getattr_str(tb[TCA_KIND]));
+ if (q) {
+ sprintf(str, "(%s) ", q->id);
+ strcat(buf, str);
+ fprintf(fp, "%s", buf);
+ buf[0] = '\0';
+ if (q->print_copt) {
+ q->print_copt(q, fp, tb[TCA_OPTIONS]);
+ }
+ }
+ if (q && show_stats)
+ {
+ int cls_indent = strlen(q->id) + 2 +
+ strlen(cls_id_str);
+ struct rtattr *xstats = NULL;
+
+ buf[0] = '\0';
+ tree_cls_newline(buf, cls, cls_indent);
+
+ if (tb[TCA_STATS] || tb[TCA_STATS2]) {
+ fprintf(fp, "\n");
+ print_tcstats_attr(fp, tb, buf, &xstats);
+ buf[0] = '\0';
+ }
+
+ if (cls->hlist.next || cls->childs_count)
+ {
+ strcat(buf, "\n");
+ tree_cls_indent(buf, cls, 1);
+ }
+ }
+ }
+ free(cls->attr);
+ fprintf(fp, "%s\n", buf);
+ buf[0] = '\0';
+
+ tree_cls_show(fp, buf, &childs, level + 1);
+ if (!cls->hlist.next) {
+ tree_cls_indent(buf, cls, 0);
+ strcat(buf, "\n");
+ }
+
+ fprintf(fp, "%s", buf);
+ buf[0] = '\0';
+ free(cls);
+ }
+}
+
int print_class(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
{
FILE *fp = (FILE*)arg;
struct tcmsg *t = NLMSG_DATA(n);
int len = n->nlmsg_len;
- struct rtattr * tb[TCA_MAX+1];
+ struct rtattr * tb[TCA_MAX+1] = {};
struct qdisc_util *q;
char abuf[256];
@@ -167,13 +322,18 @@ int print_class(const struct sockaddr_nl *who,
fprintf(stderr, "Wrong len %d\n", len);
return -1;
}
+
+ if (show_tree) {
+ tree_cls_add(t->tcm_parent, t->tcm_handle, TCA_RTA(t), len);
+ return 0;
+ }
+
if (filter_qdisc && TC_H_MAJ(t->tcm_handle^filter_qdisc))
return 0;
if (filter_classid && t->tcm_handle != filter_classid)
return 0;
- memset(tb, 0, sizeof(tb));
parse_rtattr(tb, TCA_MAX, TCA_RTA(t), len);
if (tb[TCA_KIND] == NULL) {
@@ -306,6 +466,12 @@ static int tc_class_list(int argc, char **argv)
return 1;
}
+ if (show_tree)
+ {
+ char buf[1024];
+ tree_cls_show(stdout, &buf[0], &root_cls_list, 0);
+ }
+
return 0;
}
diff --git a/tc/tc_common.h b/tc/tc_common.h
index 4f88856..0ee009b 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -19,3 +19,5 @@ extern int parse_estimator(int *p_argc, char ***p_argv, struct tc_estimator *est
struct tc_sizespec;
extern int parse_size_table(int *p_argc, char ***p_argv, struct tc_sizespec *s);
extern int check_size_table_opts(struct tc_sizespec *s);
+
+extern int show_tree;
--
2.1.3
^ permalink raw reply related
* Re: Recent Linus' tree, kernel BUG at fs/inode.c:1436!
From: Al Viro @ 2014-12-19 12:01 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, linux-fsdevel
In-Reply-To: <54940D28.8050901@parallels.com>
On Fri, Dec 19, 2014 at 02:34:00PM +0300, Pavel Emelyanov wrote:
> Hi,
>
> It looks like there's a strange refcount underflow in VFS/socket code.
> The proggie [1] crashes the recent Linus' tree (d790be38 Merge tag
> 'modules-next-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux)
> with the calltrace [2].
>
> If in the proggie the psk is replaced with non-socket descriptor the
> issue doesn't appear.
Gyah... mismerge on cherry-pick. My fault - ->i_fop assignment should've
been removed from sock_alloc_file() in bd9b51. Could you verify that the
following recovers the things?
diff --git a/net/socket.c b/net/socket.c
index 70bbde6..a2c33a4 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -372,7 +372,6 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
path.mnt = mntget(sock_mnt);
d_instantiate(path.dentry, SOCK_INODE(sock));
- SOCK_INODE(sock)->i_fop = &socket_file_ops;
file = alloc_file(&path, FMODE_READ | FMODE_WRITE,
&socket_file_ops);
^ 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