Re: [ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-10-05 Thread Pravin Shelar 
On Wed, Oct 5, 2016 at 8:58 AM, Ben Pfaff  wrote:
> On Wed, Oct 05, 2016 at 05:53:14PM +0300, Or Gerlitz wrote:
>> On 10/5/2016 3:27 AM, Ben Pfaff wrote:
>> >On Tue, Sep 27, 2016 at 03:46:04PM +0300, Paul Blakey wrote:
>> >>>Add tc ingress qdisc support so we can add qdisc
>> >>>as a qos on port or through config.
>> >>>usage:
>> >>>ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
>> >>>qos type=linux-ingress
>> >>>where  is a already added port using vsctl add-port.
>> >>>
>> >>>Signed-off-by: Paul Blakey
>> >>>Signed-off-by: Shahar Klein
>> >I don't plan to review all of these patches but this one caught my eye.
>>
>> any specail reason for not looking on this series?
>
> I was under the impression that Pravin was planning to make preliminary
> comments.  Pravin, is that true?

I am planing on reviewing it. But I have not looked it in details yet.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-10-05 Thread Ben Pfaff 
On Wed, Oct 05, 2016 at 05:53:14PM +0300, Or Gerlitz wrote:
> On 10/5/2016 3:27 AM, Ben Pfaff wrote:
> >On Tue, Sep 27, 2016 at 03:46:04PM +0300, Paul Blakey wrote:
> >>>Add tc ingress qdisc support so we can add qdisc
> >>>as a qos on port or through config.
> >>>usage:
> >>>ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
> >>>qos type=linux-ingress
> >>>where  is a already added port using vsctl add-port.
> >>>
> >>>Signed-off-by: Paul Blakey
> >>>Signed-off-by: Shahar Klein
> >I don't plan to review all of these patches but this one caught my eye.
> 
> any specail reason for not looking on this series?

I was under the impression that Pravin was planning to make preliminary
comments.  Pravin, is that true?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-10-05 Thread Or Gerlitz 

On 10/5/2016 3:27 AM, Ben Pfaff wrote:

On Tue, Sep 27, 2016 at 03:46:04PM +0300, Paul Blakey wrote:

>Add tc ingress qdisc support so we can add qdisc
>as a qos on port or through config.
>usage:
>ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
>qos type=linux-ingress
>where  is a already added port using vsctl add-port.
>
>Signed-off-by: Paul Blakey
>Signed-off-by: Shahar Klein

I don't plan to review all of these patches but this one caught my eye.


any specail reason for not looking on this series?

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-10-04 Thread Ben Pfaff 
On Tue, Sep 27, 2016 at 03:46:04PM +0300, Paul Blakey wrote:
> Add tc ingress qdisc support so we can add qdisc
> as a qos on port or through config.
> usage:
> ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
> qos type=linux-ingress
> where  is a already added port using vsctl add-port.
> 
> Signed-off-by: Paul Blakey 
> Signed-off-by: Shahar Klein 

I don't plan to review all of these patches but this one caught my eye.

I think that the ingress qdisc is different from the other qdiscs, in
that it is not an alternative to the others but can be used at the same
time as any other qdisc.  It affects how packets are treated when they
are received, rather than how they are treated as they are sent.

That's why OVS already supports the ingress qdisc, through the
ingress_policing_rate and ingress_policing_burst settings in the
Interface table.

Therefore, I think that this patch should be dropped.

Thanks,

Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-09-27 Thread Paul Blakey 
Add tc ingress qdisc support so we can add qdisc
as a qos on port or through config.
usage:
ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
qos type=linux-ingress
where  is a already added port using vsctl add-port.

Signed-off-by: Paul Blakey 
Signed-off-by: Shahar Klein 
---
 lib/netdev-linux.c | 72 ++
 1 file changed, 72 insertions(+)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 8e5fa90..5109b8c 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -419,6 +419,7 @@ static const struct tc_ops tc_ops_codel;
 static const struct tc_ops tc_ops_fqcodel;
 static const struct tc_ops tc_ops_sfq;
 static const struct tc_ops tc_ops_default;
+static const struct tc_ops tc_ops_ingress;
 static const struct tc_ops tc_ops_other;
 
 static const struct tc_ops *const tcs[] = {
@@ -428,6 +429,7 @@ static const struct tc_ops *const tcs[] = {
 _ops_fqcodel,/* Fair queue controlled delay */
 _ops_sfq,/* Stochastic fair queueing */
 _ops_default,/* Default qdisc (see tc-pfifo_fast(8)). */
+_ops_ingress,/* Ingress. */
 _ops_other,  /* Some other qdisc. */
 NULL
 };
@@ -4570,6 +4572,76 @@ static const struct tc_ops tc_ops_hfsc = {
 hfsc_class_dump_stats   /* class_dump_stats */
 };
 
+/* ingress traffic control class. */
+
+struct ingress {
+struct tc tc;
+};
+
+static void
+ingress_install__(struct netdev *netdev_)
+{
+struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+struct ingress *ingress;
+
+ingress = xmalloc(sizeof *ingress);
+tc_init(>tc, _ops_ingress);
+
+netdev->tc = >tc;
+}
+
+static void
+ingress_parse_qdisc_details__(struct netdev *netdev OVS_UNUSED,
+  const struct smap *details OVS_UNUSED, 
+  struct ingress *ingress OVS_UNUSED)
+{
+   /* parse details  */
+}
+
+static int
+ingress_tc_install(struct netdev *netdev, const struct smap *details 
OVS_UNUSED)
+{
+int error;
+struct ingress ingress;
+
+ingress_parse_qdisc_details__(netdev, details, );
+error = tc_add_del_ingress_qdisc(netdev, true);
+if (!error) {
+ingress_install__(netdev);
+}
+return error;
+}
+
+static int
+ingress_tc_load(struct netdev *netdev, struct ofpbuf *nlmsg OVS_UNUSED)
+{
+ingress_install__(netdev);
+return 0;
+}
+
+static void
+ingress_tc_destroy(struct tc *tc)
+{
+struct ingress *ingress = CONTAINER_OF(tc, struct ingress, tc);
+free(ingress);
+}
+
+static const struct tc_ops tc_ops_ingress = {
+"ingress",   /* linux_name */
+"linux-ingress", /* ovs_name */
+0,   /* n_queues */
+ingress_tc_install,
+ingress_tc_load,
+ingress_tc_destroy,
+NULL,
+NULL,
+NULL,
+NULL,
+NULL,
+NULL,
+NULL
+};
+
 /* "linux-default" traffic control class.
  *
  * This class represents the default, unnamed Linux qdisc.  It corresponds to
-- 
1.8.3.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev