[ovs-dev] Ich habe mich entschlossen, Ihnen die Summe von $ 150.000, 00 als Teil

2018-11-16 Thread Tom Crist
Grüße dich,

Ich bestätige den Erhalt Ihrer E-Mail. Ich bin Mr. Tom Crist, ein älterer 
Bürger Kanadas und in Terrace B.C. Ich zog von Terrace B.C, nachdem ich im Mai 
2013 eine Lotto-Max-Ziehung in Höhe von 40 Millionen Dollar gewonnen hatte. Ich 
ziehe von Kanada zu meinem neuen Zuhause hier in Seattle, Washington. Mein 
Jackpot war ein Geschenk Gottes an mich und ich habe mich entschieden, den 
WILLEN Gottes zu tun und auch meine gesamten 40 Millionen US-Dollar in einer 
schönen Erinnerung an meine verstorbene Frau zu spenden.

Seien Sie versichert, dass dies zu 100% legitim ist. Besuchen Sie zur 
Bestätigung die folgende Website:

http://www.calgaryherald.com/Record+breaking+lottery+winner+plans+honour+late+wife+with+donations/9292166/story.html

Ich habe mich entschlossen, Ihnen die Summe von $ 150.000,00 als Teil meines 
Neujahrs-Wohltätigkeitsprojekts zur Verfügung zu stellen, um das Leben vieler 
Menschen in Ihrer Umgebung aus meinem mit 40 Millionen Dollar dotierten 
Jackpot-Lotterie-Gewinn und der schönen Erinnerung an meine verstorbene Frau zu 
verbessern. Ich betete und suchte über das Internet nach jemandem, um dieses 
Geld in Ihrem Bundesstaat zu spenden. Ich sah Ihr Profil in der Google / 
Facebook-Liste der E-Mail-Besitzer und ich habe Sie ausgewählt.

Mein Mitgefühl für die Sorgen anderer hat sich durch die Tragödie in meinem 
Leben verstärkt. Meine Frau, Jan, starb im Februar 2012 im Alter von 57 Jahren 
an Lungenkrebs. In Erinnerung an sie habe ich mich entschlossen, meine gesamten 
40 Millionen Dollar für wohltätige Zwecke und weniger Privilegien in der 
Gesellschaft zu spenden.

Ich tue dies, um nicht gerade berühmt zu werden. Ich habe mich entschlossen, 
dies aus den Medien zu streichen, und mein einziger Zweck war, das Ergebnis 
eines langen Gesprächs mit meiner einzigen Tochter (Audrey), die mich immer 
beraten hat an Fremde in diesem neuen Jahr zu spenden, weil wir wissen, dass es 
viele Familien gibt, die von einem Gehaltsscheck leben, um den Scheck zu 
bezahlen, und anderen, die sich nicht um ihre finanziellen Verpflichtungen 
kümmern können. Obwohl ich diese Lotteriefonds im Mai 2013 gewonnen habe, haben 
wir einigen Wohltätigkeitsorganisationen, Familienmitgliedern, Fremden und 
Freunden von meinem Gewinn geholfen.

Meine Tochter (Audrey) hat Krebs und der Arzt teilte uns mit, dass sie das 
kommende Jahr vielleicht nicht mehr lange sehen werde. Sie möchte Gutes tun, 
bevor sie diese sündige Welt endgültig verlässt. Meine Tochter (Audrey) ist mir 
so lieb und sie bedeutet mir die ganze Welt. Ich habe beschlossen, ihren Wunsch 
in diesem neuen Jahr zu erfüllen. Ich möchte, dass Sie dieses Geld in ein 
lukratives Geschäft investieren, um mehr Beschäftigungsmöglichkeiten zu 
schaffen, um arbeitslosen und bedürftigen Menschen in Ihrer Nähe zu helfen, da 
wir bereits Geld an die örtliche Feuerwehr, das Rote Kreuz, Haiti, 
Krankenhäuser in Truro gespendet haben, in denen sich Audrey befindet ihre 
Krebsbehandlung und einige andere Organisationen in Asien und Europa, die 
Krebs, Alzheimer und Diabetes bekämpfen.

Um den Auszahlungsprozess des Geldes (150.000,00 $) zu erleichtern, das 
ausschließlich für Sie gespendet wurde, senden Sie mir Ihren vollständigen 
Namen, Ihre Adresse und Telefonnummer für die Auszahlung (ich bat um kurze 
Informationen, nur um zu erfahren, von wem wir dieses Geld spenden und wegen 
des Identitätsdiebstahls im Internet, den ich in der Vergangenheit zum Opfer 
gefallen bin, möchte ich Sie nicht nach Ihrem Ausweis fragen, da wir nicht den 
Eindruck haben möchten, dass ich Ihre Identität stehlen möchte ).

Ich hoffe, dass Sie die Gelder in Ihrem Bundesland sinnvoll einsetzen können. 
Wir werden Sie dafür einsetzen, dass Sie das Armutsniveau in Ihrer Region 
verringern und versuchen, den Lebensstandard von so vielen Menschen wie möglich 
zu verbessern, denn dies ist das einzige Ziel, dieses Geld an Sie zu spenden .

Wir möchten, dass Sie uns erneut versichern, dass Sie unseren Anweisungen 
folgen und meiner Tochter helfen, ihren Traum zu verwirklichen, bevor ich Ihnen 
Informationen über den Erhalt dieses Geldes erteile. Wenn Sie das Gefühl haben, 
dass Sie dieses Projekt nicht bearbeiten können, sollten Sie mich darüber 
informieren Ich kann also schnell nach einer anderen Person suchen, um das Geld 
zu spenden.

Wenn Sie Fragen haben, zögern Sie bitte nicht zu fragen.

Ich erwarte, bald von Ihnen zu hören.

Gott segne dich und herzliche Grüße

Herr Tom Crist
crist_...@aol.com

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH dpdk-latest] netdev-dpdk: Fix returning the field of malloced struct.

2018-11-16 Thread Stokes, Ian
> On 11/14/2018 01:41 PM, Ilya Maximets wrote:
> > There is no sense returning the field because it never used.
> > Function returns the pointer just to allow the caller to free it.
> > Returning the pointer to the actual structure simplifies the code
> > allowing not to explain why we're freeing the container of.
> >
> 
> Acked-by: Kevin Traynor 
> 

Thanks all, I've applied this to dpdk-latest and dpdk-hwol.

Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] OVN: add mac address only support to IPAM/MACAM

2018-11-16 Thread Lorenzo Bianconi
> Hi Lorenzo,
>
> Since I'm aware of the request from within Red Hat for this
> functionality, I'm fine with this being added in.
>
> I have some notes in-line below about the specific implementation.

Hi Mark,

thx for the review

>
> On 11/16/2018 07:00 AM, Lorenzo Bianconi wrote:
> > Add the capability to assign just the L2 address to IPAM/MACAM since
> > in the current implementation either subnet or ipv6_prefix are mandatory
> > to enable IPAM
> >
> > Signed-off-by: Lorenzo Bianconi 
> > ---
> >   ovn/northd/ovn-northd.c |  8 +++-
> >   ovn/ovn-nb.xml  |  5 +
> >   tests/ovn.at| 17 +
> >   3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 4580cd705..9e547c7b2 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -414,6 +414,7 @@ struct ipam_info {
> >   unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
> >   bool ipv6_prefix_set;
> >   struct in6_addr ipv6_prefix;
> > +bool mac_only;
> >   };
> >
> >   /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
> > @@ -559,6 +560,10 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
> >   }
> >
> >   if (!subnet_str) {
> > +if (!ipv6_prefix) {
> > +od->ipam_info.mac_only = smap_get_bool(>nbs->other_config,
> > +   "mac_only", false);
> > +}
> >   return;
> >   }
> >
> > @@ -1382,7 +1387,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
> >   const struct nbrec_logical_switch_port *nbsp = 
> > od->nbs->ports[i];
> >
> >   if (!od->ipam_info.allocated_ipv4s &&
> > -!od->ipam_info.ipv6_prefix_set) {
> > +!od->ipam_info.ipv6_prefix_set &&
> > +!od->ipam_info.mac_only) {
> >   if (nbsp->dynamic_addresses) {
> >   nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
> >   NULL);
> > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> > index 474b4f9a7..a5523ee18 100644
> > --- a/ovn/ovn-nb.xml
> > +++ b/ovn/ovn-nb.xml
> > @@ -267,6 +267,11 @@
> > 8230:5678::
> >   
> > 
> > +
> > +  
> > +Boolean value used to request to assing L2 address only if neigter 
> > subnet
> s/assing/assign/
> s/neigter/neither/
>

ack

> > +nor ipv6_prefix are specified
> > +  
> >   
> >
> >   
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index ab32faa6b..15c96976a 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -5633,6 +5633,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 
> > dynamic_addresses], [0],
> >   ["00:11:22:a8:64:05 192.168.100.4"
> >   ])
> >
> > +# request to assign mac only
> > +#
> > +ovn-nbctl ls-add sw7
> > +ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
> > +for n in $(seq 1 3); do
> > +ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" 
> > dynamic
> > +done
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
> > +["00:11:22:00:00:03"
> > +])
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
> > +["00:11:22:00:00:04"
> > +])
> > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
> > +["00:11:22:00:00:05"
> > +])
> > +
>
> Why do these MACs begin with 00:11:22? Since you didn't set a mac_prefix
> on sw7, shouldn't they start with 0a:00:00?
>

mac_prefix is global for deployment and it has been set in the previous test

Regards,
Lorenzo

> >   as ovn-sb
> >   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> >
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC] OVN: add mac address only support to IPAM/MACAM

2018-11-16 Thread Mark Michelson

Hi Lorenzo,

Since I'm aware of the request from within Red Hat for this 
functionality, I'm fine with this being added in.


I have some notes in-line below about the specific implementation.

On 11/16/2018 07:00 AM, Lorenzo Bianconi wrote:

Add the capability to assign just the L2 address to IPAM/MACAM since
in the current implementation either subnet or ipv6_prefix are mandatory
to enable IPAM

Signed-off-by: Lorenzo Bianconi 
---
  ovn/northd/ovn-northd.c |  8 +++-
  ovn/ovn-nb.xml  |  5 +
  tests/ovn.at| 17 +
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4580cd705..9e547c7b2 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -414,6 +414,7 @@ struct ipam_info {
  unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
  bool ipv6_prefix_set;
  struct in6_addr ipv6_prefix;
+bool mac_only;
  };
  
  /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or

@@ -559,6 +560,10 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
  }
  
  if (!subnet_str) {

+if (!ipv6_prefix) {
+od->ipam_info.mac_only = smap_get_bool(>nbs->other_config,
+   "mac_only", false);
+}
  return;
  }
  
@@ -1382,7 +1387,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)

  const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
  
  if (!od->ipam_info.allocated_ipv4s &&

-!od->ipam_info.ipv6_prefix_set) {
+!od->ipam_info.ipv6_prefix_set &&
+!od->ipam_info.mac_only) {
  if (nbsp->dynamic_addresses) {
  nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
  NULL);
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 474b4f9a7..a5523ee18 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -267,6 +267,11 @@
8230:5678::
  

+
+  
+Boolean value used to request to assing L2 address only if neigter 
subnet

s/assing/assign/
s/neigter/neither/


+nor ipv6_prefix are specified
+  
  
  
  

diff --git a/tests/ovn.at b/tests/ovn.at
index ab32faa6b..15c96976a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5633,6 +5633,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 
dynamic_addresses], [0],
  ["00:11:22:a8:64:05 192.168.100.4"
  ])
  
+# request to assign mac only

+#
+ovn-nbctl ls-add sw7
+ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
+for n in $(seq 1 3); do
+ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" dynamic
+done
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
+["00:11:22:00:00:03"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
+["00:11:22:00:00:04"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
+["00:11:22:00:00:05"
+])
+


Why do these MACs begin with 00:11:22? Since you didn't set a mac_prefix 
on sw7, shouldn't they start with 0a:00:00?



  as ovn-sb
  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
  



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] OVN: add selected mac address to MACAM in update_dynamic_addresses

2018-11-16 Thread Mark Michelson

Acked-by: Mark Michelson 

On 11/16/2018 06:31 AM, Lorenzo Bianconi wrote:

Add selected dynamic mac address to MACAM in update_dynamic_addresses
and not just in in ipam_add_port_addresses/ipam_insert_lsp_addresses
since the second approach can produce a duplicated L2 address in a
IPv6-only network if ipv6_prefix is provided after logical port creation.
The issue can be triggered with the following reproducer:

$ovn-nbctl ls-add sw0
$ovn-nbctl lsp-add sw0 sw0-port1
$ovn-nbctl lsp-set-addresses sw0-port1 "dynamic"
$ovn-nbctl lsp-add sw0 sw0-port2
$ovn-nbctl lsp-set-addresses sw0-port2 "dynamic"
$ovs-vsctl add-port br-int p1 -- \
 set Interface p1 external_ids:iface-id=sw0-port1
$ovs-vsctl add-port br-int p2 -- \
 set Interface p2 external_ids:iface-id=sw0-port2
[..]
$ovn-nbctl --wait=sb set Logical-switch sw0 \
 other_config:ipv6_prefix="aef0::"

$ovn-nbctl list logical_switch_port
_uuid   : 1e0e2ed8-20c6-48dc-bfa8-d823e48c9f45
addresses   : [dynamic]
dhcpv4_options  : []
dhcpv6_options  : []
dynamic_addresses   : "0a:00:00:00:00:01 aef0::800:ff:fe00:1"
enabled : []
external_ids: {}
name: "sw0-port1"
options : {}
parent_name : []
port_security   : []
tag : []
tag_request : []
type: ""
up  : true

_uuid   : cfeab7fb-e20b-41f1-974c-f99e0b5293d7
addresses   : [dynamic]
dhcpv4_options  : []
dhcpv6_options  : []
dynamic_addresses   : "0a:00:00:00:00:01 aef0::800:ff:fe00:1"
enabled : []
external_ids: {}
name: "sw0-port2"
options : {}
parent_name : []
port_security   : []
tag : []
tag_request : []
type: ""
up  : true

Fixes: c814545b43ac ("OVN: configure L2 address according to the used IP
address")

Signed-off-by: Lorenzo Bianconi 
---
  ovn/northd/ovn-northd.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 58bef7de5..4580cd705 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1341,6 +1341,8 @@ update_dynamic_addresses(struct dynamic_address_update 
*update)
  
  struct ds new_addr = DS_EMPTY_INITIALIZER;

  ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+ipam_insert_mac(, true);
+
  if (ip4) {
  ipam_insert_ip(update->od, ntohl(ip4));
  ds_put_format(_addr, " "IP_FMT, IP_ARGS(ip4));



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] Possible OVN enhancement: implementing OpenShift's "unidling" behavior

2018-11-16 Thread Guru Shetty
I don't have specific comments. But I do believe that "holding" packet may
not work well as the time taken to create new pods, assign IPs to those
pod, create new load-balancer rules for the new pods, will be in "seconds".


On Fri, 9 Nov 2018 at 12:26, Mark Michelson  wrote:

> Yesterday during the OVN IRC meeting, I brought up a subject that
> requires a bit more discussion.
>
> The issue: OpenShift has a use case where it uses a load balancer. The
> VIP of the load balancer is backed with k8s pods. During times of
> inactivity, these pods may be deleted due to idleness. Eventually, all
> backing pods may be removed due to idleness. What OpenShift wants is
> when a packet arrives for the load balancer VIP in this scenario, they
> want to be able to react by creating new backing pods, updating load
> balancer configuration, and reprocessing the packet so that it will
> reach a destination pod.
>
>  From an OVS/OVN perspective, what needs to happen is that based on an
> incoming packet, an outside party needs to be notified to take an
> action. The interesting part is at what layer the third party is
> notified and how the incoming packet is further handled. We've discussed
> this within Red Hat and have three ideas for how to do this.
>
> 1) The database approach
> We add a new table to the southbound database, perhaps called
> CMS_Events. In the situation where OpenShift wants unidling behavior, it
> sets an option that results in ovn-controller installing a flow in OVS
> to send the incoming packet to ovn-controller. ovn-controller, upon
> receiving the incoming packet, buffers the packet and puts a new row in
> the CMS_Events table. The CMS, upon seeing the event, takes its
> necessary action. In this particular case, the CMS action is to spin up
> pods and alter load balancer configuration. The CMS then sets a value in
> row to indicate it has handled the event. ovn-controller, seeing the
> event is handled, deletes the event and continues processing the packet,
> allowing it to reach its destination.
>
> 2) The message passing approach
> In the situation where OpenShift wants unidling behavior, it sets an
> option that results in ovn-controller installing a flow in OVS to send
> the incoming packet to ovn-controller. ovn-controller, upon receiving
> the packet, buffers the packet and sends some sort of message to the
> CMS. The contents of this message and the method by which the message is
> sent would likely be dependent on the CMS in use. The CMS would act on
> this message, spinning up new pods and altering load balancer
> configuration. The CMS would then send a message back to ovn-controller,
> where ovn-controller would continue processing the packet, allowing it
> to reach its destination.
>
> 3) The second controller approach
> In the situation where OpenShift wants unidling behavior, it sets an
> option that results in ovn-controller installing a flow in OVS to send
> the incoming packet to an external controller. The external controller,
> upon receiving the packet, spins up new pods and alters load balancer
> configuration. The external controller then (somehow) sends the packet
> back into OVS so it can reach its destination.
>
> I presented the database approach during the meeting yesterday, but it's
> worth presenting the other ideas we've discussed. Does any one of these
> three ideas jump out as being significantly better than the others? Do
> you have other ideas for how an external service can be contacted based
> on the reception of a packet destined for a certain IP address?
>
> Thanks,
> Mark Michelson
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap-file: Correctly format enum type.

2018-11-16 Thread Ben Pfaff
On Fri, Nov 16, 2018 at 10:21:22AM -0800, Simon Horman wrote:
> On Fri, Nov 16, 2018 at 09:25:11AM -0800, Ben Pfaff wrote:
> > The underlying type for an enum is somewhat unpredictable in that the
> > compiler and the ABI influence it.  The format specifier I used here was
> > apparently correct for i386 on Linux but wrong for x86-64.  It's better to
> > just use a cast.
> > 
> > Reported-by: Simon Horman 
> > Signed-off-by: Ben Pfaff 
> 
> Thanks Ben,
> 
> this looks good to me.
> 
> Reviewed-by: Simon Horman 

Thanks, I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap-file: Correctly format enum type.

2018-11-16 Thread Simon Horman
On Fri, Nov 16, 2018 at 09:25:11AM -0800, Ben Pfaff wrote:
> The underlying type for an enum is somewhat unpredictable in that the
> compiler and the ABI influence it.  The format specifier I used here was
> apparently correct for i386 on Linux but wrong for x86-64.  It's better to
> just use a cast.
> 
> Reported-by: Simon Horman 
> Signed-off-by: Ben Pfaff 

Thanks Ben,

this looks good to me.

Reviewed-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] pinctrl: Check requested IP in DHCPREQUEST messages

2018-11-16 Thread Gregory Smith
See RFC 2131, section 4.3.2. When handling a DHCPREQUEST message, the
server should validate that the client's requested IP matches the
offered IP. If not, the server should reply with a DHCPNAK. The client's
requested IP is either specified as the Requested IP Address (option
50), or as the ciaddr, depending on the client's state.

Signed-off-by: Gregory Smith 
---
 lib/dhcp.h   |   3 +
 ovn/controller/pinctrl.c |  79 +---
 tests/ovn.at | 188 +--
 3 files changed, 220 insertions(+), 50 deletions(-)

diff --git a/lib/dhcp.h b/lib/dhcp.h
index 271e0a5..73f593a 100644
--- a/lib/dhcp.h
+++ b/lib/dhcp.h
@@ -54,7 +54,10 @@ BUILD_ASSERT_DECL(DHCP_HEADER_LEN == sizeof(struct 
dhcp_header));
 #define DHCP_MSG_OFFER 2
 #define DHCP_MSG_REQUEST   3
 #define DHCP_MSG_ACK   5
+#define DHCP_MSG_NAK   6
 
+#define DHCP_OPT_PAD   0
+#define DHCP_OPT_REQ_IP50
 #define DHCP_OPT_MSG_TYPE  53
 #define DHCP_OPT_END   255
 
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 56539a8..2c1a380 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -608,8 +608,11 @@ pinctrl_handle_put_dhcp_opts(
  *| UDP HEADER  | DHCP HEADER  | 4 Byte DHCP Cookie | DHCP OPTIONS(var 
len)|
  * 
  */
-if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN +
-sizeof (struct dhcp_header) + sizeof(uint32_t) + 3)) {
+
+size_t pkt_in_l4_size = dp_packet_l4_size(pkt_in);
+size_t req_l4_size = (UDP_HEADER_LEN +
+  sizeof (struct dhcp_header) + sizeof(uint32_t) + 3);
+if (pkt_in_l4_size < req_l4_size) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 VLOG_WARN_RL(, "Invalid or incomplete DHCP packet received");
 goto exit;
@@ -655,12 +658,58 @@ pinctrl_handle_put_dhcp_opts(
 if (in_dhcp_opt[2] == DHCP_MSG_DISCOVER) {
 msg_type = DHCP_MSG_OFFER;
 } else {
+/* This is a DHCPREQUEST. If the client has requested an IP that
+ * does not match the offered IP address, reply with a NAK. The
+ * requested IP address may be supplied either via Requested IP Address
+ * (opt 50) or via ciaddr, depending on the client's state.
+ */
+ovs_be32 *request_ip = NULL;
+in_dhcp_opt += 3;
+while (!request_ip) {
+req_l4_size += 1;
+if (pkt_in_l4_size < req_l4_size ||
+in_dhcp_opt[0] == DHCP_OPT_END) {
+break;
+}
+if (in_dhcp_opt[0] == DHCP_OPT_PAD) {
+in_dhcp_opt += 1;
+continue;
+}
+req_l4_size += 1;
+if (pkt_in_l4_size < req_l4_size) {
+break;
+}
+req_l4_size += in_dhcp_opt[1];
+if (pkt_in_l4_size < req_l4_size) {
+break;
+}
+if (in_dhcp_opt[0] == DHCP_OPT_REQ_IP && in_dhcp_opt[1] == 4) {
+request_ip = (ovs_be32 *)_dhcp_opt[2];
+}
+in_dhcp_opt += 2 + in_dhcp_opt[1];
+}
 msg_type = DHCP_MSG_ACK;
+if (request_ip) {
+if (*request_ip != *offer_ip) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "DHCPREQUEST requested IP "IP_FMT" does not "
+ "match offer "IP_FMT, IP_ARGS(*request_ip),
+ IP_ARGS(*offer_ip));
+msg_type = DHCP_MSG_NAK;
+}
+} else if (in_dhcp_data->ciaddr != *offer_ip) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "DHCPREQUEST ciaddr "IP_FMT" does not match "
+ "offer "IP_FMT, IP_ARGS(in_dhcp_data->ciaddr),
+ IP_ARGS(*offer_ip));
+msg_type = DHCP_MSG_NAK;
+}
 }
 
 /* Frame the DHCP reply packet
  * Total DHCP options length will be options stored in the userdata +
- * 16 bytes.
+ * 16 bytes. Note that the DHCP options stored in userdata are not included
+ * in DHCPNAK messages.
  *
  * --
  *| 4 Bytes (dhcp cookie) | 3 Bytes (option type) | DHCP options |
@@ -668,8 +717,10 @@ pinctrl_handle_put_dhcp_opts(
  *| 4 Bytes padding | 1 Byte (option end 0xFF ) | 4 Bytes padding|
  * --
  */
-uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + \
-   userdata->size + 16;
+uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
+if (msg_type != DHCP_MSG_NAK) {
+new_l4_size += userdata->size;
+}
 size_t new_packet_size = pkt_in->l4_ofs + new_l4_size;
 
 struct dp_packet 

Re: [ovs-dev] [PATCH] pcap-file: Add support for Linux SLL formatted PCAP files.

2018-11-16 Thread Ben Pfaff
On Fri, Nov 16, 2018 at 08:44:10AM -0800, Simon Horman wrote:
> On Sun, Nov 11, 2018 at 03:41:08PM -0800, Ben Pfaff wrote:
> > Someone sent me one of these and OVS couldn't read it.  This fixes the
> > problem.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/pcap-file.c | 54 ++
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/lib/pcap-file.c b/lib/pcap-file.c
> > index 81a094c2a01f..0f34c5e83cda 100644
> > --- a/lib/pcap-file.c
> > +++ b/lib/pcap-file.c
> > @@ -39,9 +39,15 @@ enum ts_resolution {
> >  PCAP_NSEC,
> >  };
> >  
> > +enum network_type {
> > +PCAP_ETHERNET = 0,
> > +PCAP_LINUX_SLL = 0x71
> > +};
> > +
> >  struct pcap_file {
> >  FILE *file;
> >  enum ts_resolution resolution;
> > +enum network_type network;
> >  };
> >  
> >  struct pcap_hdr {
> > @@ -130,10 +136,13 @@ ovs_pcap_read_header(struct pcap_file *p_file)
> >  VLOG_WARN("failed to read pcap header: %s", 
> > ovs_retval_to_string(error));
> >  return error;
> >  }
> > +bool byte_swap;
> >  if (ph.magic_number == 0xa1b2c3d4 || ph.magic_number == 0xd4c3b2a1) {
> > +byte_swap = ph.magic_number == 0xd4c3b2a1;
> >  p_file->resolution = PCAP_USEC;
> >  } else if (ph.magic_number == 0xa1b23c4d ||
> > ph.magic_number == 0x4d3cb2a1) {
> > +byte_swap = ph.magic_number == 0x4d3cb2a1;
> >  p_file->resolution = PCAP_NSEC;
> >  } else {
> >  VLOG_WARN("bad magic 0x%08"PRIx32" reading pcap file "
> > @@ -141,6 +150,13 @@ ovs_pcap_read_header(struct pcap_file *p_file)
> >"or 0x4d3cb2a1)", ph.magic_number);
> >  return EPROTO;
> >  }
> > +p_file->network = byte_swap ? uint32_byteswap(ph.network) : ph.network;
> > +if (p_file->network != PCAP_ETHERNET &&
> > +p_file->network != PCAP_LINUX_SLL) {
> > +VLOG_WARN("unknown network type %"PRIu16" reading pcap file",
> > +  p_file->network);
> 
> Hi Ben,
> 
> the line above seems to produce an error in travis-ci.
> 
> As per https://travis-ci.org/openvswitch/ovs/jobs/456022920
> 
> lib/pcap-file.c:157:19: error: format specifies type 'unsigned short' but the 
> argument has underlying type 'unsigned int' [-Werror,-Wformat]

Thanks for the report.  I sent a fix.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap-file: Add support for Linux SLL formatted PCAP files.

2018-11-16 Thread Simon Horman
On Sun, Nov 11, 2018 at 03:41:08PM -0800, Ben Pfaff wrote:
> Someone sent me one of these and OVS couldn't read it.  This fixes the
> problem.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  lib/pcap-file.c | 54 ++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/lib/pcap-file.c b/lib/pcap-file.c
> index 81a094c2a01f..0f34c5e83cda 100644
> --- a/lib/pcap-file.c
> +++ b/lib/pcap-file.c
> @@ -39,9 +39,15 @@ enum ts_resolution {
>  PCAP_NSEC,
>  };
>  
> +enum network_type {
> +PCAP_ETHERNET = 0,
> +PCAP_LINUX_SLL = 0x71
> +};
> +
>  struct pcap_file {
>  FILE *file;
>  enum ts_resolution resolution;
> +enum network_type network;
>  };
>  
>  struct pcap_hdr {
> @@ -130,10 +136,13 @@ ovs_pcap_read_header(struct pcap_file *p_file)
>  VLOG_WARN("failed to read pcap header: %s", 
> ovs_retval_to_string(error));
>  return error;
>  }
> +bool byte_swap;
>  if (ph.magic_number == 0xa1b2c3d4 || ph.magic_number == 0xd4c3b2a1) {
> +byte_swap = ph.magic_number == 0xd4c3b2a1;
>  p_file->resolution = PCAP_USEC;
>  } else if (ph.magic_number == 0xa1b23c4d ||
> ph.magic_number == 0x4d3cb2a1) {
> +byte_swap = ph.magic_number == 0x4d3cb2a1;
>  p_file->resolution = PCAP_NSEC;
>  } else {
>  VLOG_WARN("bad magic 0x%08"PRIx32" reading pcap file "
> @@ -141,6 +150,13 @@ ovs_pcap_read_header(struct pcap_file *p_file)
>"or 0x4d3cb2a1)", ph.magic_number);
>  return EPROTO;
>  }
> +p_file->network = byte_swap ? uint32_byteswap(ph.network) : ph.network;
> +if (p_file->network != PCAP_ETHERNET &&
> +p_file->network != PCAP_LINUX_SLL) {
> +VLOG_WARN("unknown network type %"PRIu16" reading pcap file",
> +  p_file->network);

Hi Ben,

the line above seems to produce an error in travis-ci.

As per https://travis-ci.org/openvswitch/ovs/jobs/456022920

lib/pcap-file.c:157:19: error: format specifies type 'unsigned short' but the 
argument has underlying type 'unsigned int' [-Werror,-Wformat]
  p_file->network);
~~^~~
./include/openvswitch/vlog.h:203:39: note: expanded from macro 'VLOG_WARN'
#define VLOG_WARN(...) VLOG(VLL_WARN, __VA_ARGS__)
   ~~~^~~
./include/openvswitch/vlog.h:277:41: note: expanded from macro 'VLOG'
vlog(_module, level__, __VA_ARGS__);   \
^~~
1 error generated.
make[2]: *** [lib/pcap-file.lo] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
The command "./.travis/${TRAVIS_OS_NAME}-build.sh $OPTS" exited with 2.



> +return EPROTO;
> +}
>  return 0;
>  }
>  
> @@ -218,6 +234,44 @@ ovs_pcap_read(struct pcap_file *p_file, struct dp_packet 
> **bufp,
>  dp_packet_delete(buf);
>  return error;
>  }
> +
> +if (p_file->network == PCAP_LINUX_SLL) {
> +/* This format doesn't include the destination Ethernet address, 
> which
> + * is weird. */
> +
> +struct sll_header {
> +ovs_be16 packet_type;
> +ovs_be16 arp_hrd;
> +ovs_be16 lla_len;
> +struct eth_addr dl_src;
> +ovs_be16 reserved;
> +ovs_be16 protocol;
> +};
> +const struct sll_header *sll;
> +if (len < sizeof *sll) {
> +VLOG_WARN("pcap packet too short for SLL header");
> +dp_packet_delete(buf);
> +return EPROTO;
> +}
> +
> +/* Pull Linux SLL header. */
> +sll = dp_packet_pull(buf, sizeof *sll);
> +if (sll->lla_len != htons(6)) {
> +ovs_hex_dump(stdout, sll, sizeof *sll, 0, false);
> +VLOG_WARN("bad SLL header");
> +dp_packet_delete(buf);
> +return EPROTO;
> +}
> +
> +/* Push Ethernet header. */
> +struct eth_header eth = {
> +/* eth_dst is all zeros because the format doesn't include it. */
> +.eth_src = sll->dl_src,
> +.eth_type = sll->protocol,
> +};
> +dp_packet_push(buf, , sizeof eth);
> +}
> +
>  *bufp = buf;
>  return 0;
>  }
> -- 
> 2.16.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] Windows: Fix broken kernel userspace communication

2018-11-16 Thread Alin Serdean
Thanks Ben! Applied on master and backported.

--
Alin

> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Friday, November 16, 2018 4:36 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v2] Windows: Fix broken kernel userspace
> communication
> 
> On Fri, Nov 16, 2018 at 03:32:58PM +0200, Alin Gabriel Serdean wrote:
> > Patch:
> >
> https://github.com/openvswitch/ovs/commit/69c51582ff786a68fc325c1c506
> 2
> > 4715482bc460 broke Windows userpace - kernel communication.
> >
> > On windows we create netlink sockets when the handlers are initiated
> > and reuse them.
> > This patch remaps the usage of the netlink socket pool.
> >
> > Fixes:
> > https://github.com/openvswitch/ovs-issues/issues/164
> >
> > Signed-off-by: Alin Gabriel Serdean 
> > Acked-by: Shashank Ram 
> > Tested-by: Shashank Ram 
> > Signed-off-by: Ben Pfaff 
> > Co-authored-by: Ben Pfaff 
> > ---
> > v1 -> v2: Fold in nice incremental from Ben RFC -> v1: Improve
> > wording, change function name and add ifdef guard for them
> 
> Looks good to me, thank you!
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] netdev-dpdk: Add mbuf HEADROOM after alignment.

2018-11-16 Thread Tiago Lam
Commit dfaf00e started using the result of dpdk_buf_size() to calculate
the available size on each mbuf, as opposed to using the previous
MBUF_SIZE macro. However, this was calculating the mbuf size by adding
up the MTU with RTE_PKTMBUF_HEADROOM and only then aligning to
NETDEV_DPDK_MBUF_ALIGN. Instead, the accounting for the
RTE_PKTMBUF_HEADROOM should only happen after alignment, as per below.

Before alignment:
ROUNDUP(MTU(1500) + RTE_PKTMBUF_HEADROOM(128), 1024) = 2048

After aligment:
ROUNDUP(MTU(1500), 1024) + 128 = 2176

This might seem insignificant, however, it might have performance
implications in DPDK, where each mbuf is expected to have 2k +
RTE_PKTMBUF_HEADROOM of available space. This is because not only some
NICs have course grained alignments of 1k, they will also take
RTE_PKTMBUF_HEADROOM bytes from the overall available space in an mbuf
when setting up their Rx requirements. Thus, only the "After alignment"
case above would guarantee a 2k of available room, as the "Before
alignment" would report only 1920B.

Some extra information can be found at:
https://mails.dpdk.org/archives/dev/2018-November/119219.html

Note: This has been found by Ian Stokes while going through some
af_packet checks.

Reported-by: Ian Stokes 
Fixes: dfaf00e ("netdev-dpdk: fix mbuf sizing")
Signed-off-by: Tiago Lam 
---
 Documentation/topics/dpdk/memory.rst | 20 ++--
 lib/netdev-dpdk.c|  6 --
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/Documentation/topics/dpdk/memory.rst 
b/Documentation/topics/dpdk/memory.rst
index c9b739f..3c4ee17 100644
--- a/Documentation/topics/dpdk/memory.rst
+++ b/Documentation/topics/dpdk/memory.rst
@@ -107,8 +107,8 @@ Example 1
 
  MTU = 1500 Bytes
  Number of mbufs = 262144
- Mbuf size = 2752 Bytes
- Memory required = 262144 * 2752 = 721 MB
+ Mbuf size = 2880 Bytes
+ Memory required = 262144 * 2880 = 755 MB
 
 Example 2
 +
@@ -116,8 +116,8 @@ Example 2
 
  MTU = 1800 Bytes
  Number of mbufs = 262144
- Mbuf size = 2752 Bytes
- Memory required = 262144 * 2752 = 721 MB
+ Mbuf size = 2880 Bytes
+ Memory required = 262144 * 2880 = 755 MB
 
 .. note::
 
@@ -130,8 +130,8 @@ Example 3
 
  MTU = 6000 Bytes
  Number of mbufs = 262144
- Mbuf size = 8000 Bytes
- Memory required = 262144 * 8000 = 2097 MB
+ Mbuf size = 6976 Bytes
+ Memory required = 262144 * 6976 = 1829 MB
 
 Example 4
 +
@@ -194,8 +194,8 @@ Example 1: (1 rxq, 1 PMD, 1500 MTU)
 
  MTU = 1500
  Number of mbufs = (1 * 2048) + (2 * 2048) + (1 * 32) + (16384) = 22560
- Mbuf size = 2752 Bytes
- Memory required = 22560 * 2752 = 62 MB
+ Mbuf size = 2880 Bytes
+ Memory required = 22560 * 2880 = 65 MB
 
 Example 2: (1 rxq, 2 PMD, 6000 MTU)
 +++
@@ -203,8 +203,8 @@ Example 2: (1 rxq, 2 PMD, 6000 MTU)
 
  MTU = 6000
  Number of mbufs = (1 * 2048) + (3 * 2048) + (1 * 32) + (16384) = 24608
- Mbuf size = 8000 Bytes
- Memory required = 24608 * 8000 = 196 MB
+ Mbuf size = 6976 Bytes
+ Memory required = 24608 * 6976 = 171 MB
 
 Example 3: (2 rxq, 2 PMD, 9000 MTU)
 +++
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e8618a6..a871743 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -521,8 +521,8 @@ is_dpdk_class(const struct netdev_class *class)
 static uint32_t
 dpdk_buf_size(int mtu)
 {
-return ROUND_UP((MTU_TO_MAX_FRAME_LEN(mtu) + RTE_PKTMBUF_HEADROOM),
- NETDEV_DPDK_MBUF_ALIGN);
+return ROUND_UP(MTU_TO_MAX_FRAME_LEN(mtu), NETDEV_DPDK_MBUF_ALIGN)
++ RTE_PKTMBUF_HEADROOM;
 }
 
 /* Allocates an area of 'sz' bytes from DPDK.  The memory is zero'ed.
@@ -681,6 +681,8 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
per_port_mp)
   dev->requested_n_rxq, dev->requested_n_txq,
   RTE_CACHE_LINE_SIZE);
 
+/* The size of the mbuf's private area (i.e. area that holds OvS'
+ * dp_packet data)*/
 mbuf_priv_data_len = sizeof(struct dp_packet) -
  sizeof(struct rte_mbuf);
 /* The size of the entire dp_packet. */
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [branch-2.8/2.9 backport] netdev-tc-offloads: Delete ufid tc mapping in the right place

2018-11-16 Thread Simon Horman
On Tue, Nov 13, 2018 at 08:36:04PM -0500, Chris Mi wrote:
> Currently, the ufid tc mapping is deleted in add_ufid_tc_mapping().
> But if tc_replace_flower() failed, the old ufid tc mapping will not
> be deleted. If another thread adds the same tc mapping successfully,
> then there will be multiple mappings for the same ifindex, handle
> and prio.
> 
> Fixes: 9116730db ("netdev-tc-offloads: Add ufid to tc/netdev map")
> Signed-off-by: Chris Mi 
> Reviewed-by: Roi Dayan 

Thanks Chris,

applied to branch-2.8 and branch-2.9.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] coding-style: Few visual enhancements for the document.

2018-11-16 Thread Ben Pfaff
On Fri, Nov 16, 2018 at 06:13:07PM +0300, Ilya Maximets wrote:
> Some keywords and numbers highlighted. Added few spaces to
> the examples.
> 
> Signed-off-by: Ilya Maximets 

Thanks!  Applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] coding-style: Few visual enhancements for the document.

2018-11-16 Thread Ilya Maximets
Some keywords and numbers highlighted. Added few spaces to
the examples.

Signed-off-by: Ilya Maximets 
---
 .../internals/contributing/coding-style.rst   | 100 +-
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/Documentation/internals/contributing/coding-style.rst 
b/Documentation/internals/contributing/coding-style.rst
index 28cbec28b..f70f783ad 100644
--- a/Documentation/internals/contributing/coding-style.rst
+++ b/Documentation/internals/contributing/coding-style.rst
@@ -131,8 +131,8 @@ Use ``XXX`` or ``FIXME`` comments to mark code that needs 
work.
 
 Don't use ``//`` comments.
 
-Don't comment out or #if 0 out code. Just remove it. The code that was there
-will still be in version control history.
+Don't comment out or ``#if 0`` out code. Just remove it. The code that was
+there will still be in version control history.
 
 .. _functions:
 
@@ -144,8 +144,8 @@ code on separate lines, all starting in column 0.
 
 Before each function definition, write a comment that describes the function's
 purpose, including each parameter, the return value, and side effects.
-References to argument names should be given in single-quotes, e.g. 'arg'. The
-comment should not include the function name, nor need it follow any formal
+References to argument names should be given in single-quotes, e.g. ``'arg'``.
+The comment should not include the function name, nor need it follow any formal
 structure. The comment does not need to describe how a function does its work,
 unless this information is needed to use the function correctly (this is often
 better done with comments *inside* the function).
@@ -174,7 +174,7 @@ In the absence of good reasons for another order, the 
following parameter order
 is preferred. One notable exception is that data parameters and their
 corresponding size parameters should be paired.
 
-1. The primary object being manipulated, if any (equivalent to the "this"
+1. The primary object being manipulated, if any (equivalent to the ``this``
pointer in C++).
 
 2. Input-only parameters.
@@ -211,8 +211,8 @@ null-pointer check. We find that this usually makes code 
easier to read.
 
 Functions in ``.c`` files should not normally be marked ``inline``, because it
 does not usually help code generation and it does suppress compiler warnings
-about unused functions. (Functions defined in .h usually should be marked
-inline.)
+about unused functions. (Functions defined in ``.h`` usually should be marked
+``inline``.)
 
 .. _function prototypes:
 
@@ -303,13 +303,13 @@ style:
 
 Use ``for (;;)`` to write an infinite loop.
 
-In an if/else construct where one branch is the "normal" or "common" case and
-the other branch is the "uncommon" or "error" case, put the common case after
-the "if", not the "else". This is a form of documentation. It also places the
-most important code in sequential order without forcing the reader to visually
-skip past less important details. (Some compilers also assume that the "if"
-branch is the more common case, so this can be a real form of optimization as
-well.)
+In an ``if/else`` construct where one branch is the "normal" or "common" case
+and the other branch is the "uncommon" or "error" case, put the common case
+after the ``if``, not the ``else``. This is a form of documentation. It also
+places the most important code in sequential order without forcing the reader
+to visually skip past less important details. (Some compilers also assume that
+the ``if`` branch is the more common case, so this can be a real form of
+optimization as well.)
 
 Return Values
 -
@@ -317,21 +317,21 @@ Return Values
 For functions that return a success or failure indication, prefer one of the
 following return value conventions:
 
-- An ``int`` where 0 indicates success and a positive errno value indicates a
-  reason for failure.
+- An ``int`` where ``0`` indicates success and a positive errno value indicates
+  a reason for failure.
 
-- A ``bool`` where true indicates success and false indicates failure.
+- A ``bool`` where ``true`` indicates success and ``false`` indicates failure.
 
 Macros
 --
 
 Don't define an object-like macro if an enum can be used instead.
 
-Don't define a function-like macro if a "static inline" function can be used
+Don't define a function-like macro if a ``static inline`` function can be used
 instead.
 
-If a macro's definition contains multiple statements, enclose them with ``do {
-... } while (0)`` to allow them to work properly in all syntactic
+If a macro's definition contains multiple statements, enclose them with
+``do { ... } while (0)`` to allow them to work properly in all syntactic
 circumstances.
 
 Do use macros to eliminate the need to update different parts of a single file
@@ -389,7 +389,7 @@ The comment should explain how the code in the file relates 
to code in other
 files. The goal is to allow a programmer to quickly figure out where a given
 module fits into the larger system.
 

Re: [ovs-dev] [PATCH v2] Windows: Fix broken kernel userspace communication

2018-11-16 Thread Ben Pfaff
On Fri, Nov 16, 2018 at 03:32:58PM +0200, Alin Gabriel Serdean wrote:
> Patch: 
> https://github.com/openvswitch/ovs/commit/69c51582ff786a68fc325c1c50624715482bc460
> broke Windows userpace - kernel communication.
> 
> On windows we create netlink sockets when the handlers are initiated and
> reuse them.
> This patch remaps the usage of the netlink socket pool.
> 
> Fixes:
> https://github.com/openvswitch/ovs-issues/issues/164
> 
> Signed-off-by: Alin Gabriel Serdean 
> Acked-by: Shashank Ram 
> Tested-by: Shashank Ram 
> Signed-off-by: Ben Pfaff 
> Co-authored-by: Ben Pfaff 
> ---
> v1 -> v2: Fold in nice incremental from Ben
> RFC -> v1: Improve wording, change function name and add ifdef guard for them

Looks good to me, thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [dpdk-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Wiles, Keith



> On Nov 16, 2018, at 3:51 AM, Ananyev, Konstantin 
>  wrote:
> 
> Hi everyone,
> 
>> 
>> Hi,
>> 
>> 16/11/2018 09:42, Ilya Maximets:
>>> Hi,
>>> While discussing the ways to enable DPDK 18.11 new features in OVS
>>> there was suggestions to use 'rte_eth_devices[]' array directly.
>>> But this array is marked as '@internal' and also it located in
>>> the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
>>> following disclaimer:
>>> 
>>> /**
>>> * @file
>>> *
>>> * RTE Ethernet Device internal header.
>>> *
>>> * This header contains internal data types. But they are still part of the
>>> * public API because they are used by inline functions in the published API.
>>> *
>>> * Applications should not use these directly.
>>> *
>>> */
>>> 
>>> From the other hand, test-pmd and some example apps in DPDK source
>>> tree are using this array for various reasons.
>>> 
>>> So, is it OK to use this array directly or not?
>> 
>> Good question :)
>> Thanks for bringing this discussion.
>> 
>> As you said, it is public because of inline functions using it directly
>> for performance purpose. The DPDK API is bad for separating public and
>> internal stuff. And over time, there is not a lot of attention on trying
>> to not use internal symbols in applications.
>> 
>>> In general we need to change the API, i.e. make 'rte_eth_devices' part
>>> of a public API. Or change the test-pmd and example apps to stop
>>> using it.
>> 
>> I agree we need to decide an option and make it clear.
>> 
>> We can try to make this variable private and add more public functions
>> to use it (I'm thinking at more iterators like sibling ones).
>> It would clarify the API.
>> It can be evaluated what is the real cost after compiler optimization
>> for Rx/Tx functions. It can also be evaluated to uninline functions.
>> 
>> On the other hand, we can wonder what is the real benefit of trying to
>> hide access to internal resources. Should we make all public?
> 
> In that case every change in any of such structures will be an ABI breakage.
> Even now any change in rte_eth_dev is quite problematic because of that.
> I think we better keep them private as much as possible and cleanup
> our examples and testpmd code.
> Konstantin

I Agree here, I have noticed a few places we allow direct access to internal 
data structures, which we need to restrict access by making them private with 
getter/setter functions or just getter/setter functions even if we can not make 
them private. At least with moving members and adding members we can state that 
it is not a ABI breakage as long as everyone uses the getter/setter functions. 
These functions could not be inline functions correct as that would still break 
API?

> 
>> 
>>> One more related question: Is it OK to access internal device
>>> stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
>>> This looks really dangerous. It's unclear why pointers like this
>>> exposed to user.
>> 
>> It's a lot easier to expose pointers than doing a good API for all uses.
>> We need to question what is really dangerous and what we want to avoid?

Regards,
Keith

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [dpdk-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Wiles, Keith



On Nov 16, 2018, at 3:51 AM, Ananyev, Konstantin 
mailto:konstantin.anan...@intel.com>> wrote:

Hi everyone,


Hi,

16/11/2018 09:42, Ilya Maximets:
Hi,
While discussing the ways to enable DPDK 18.11 new features in OVS
there was suggestions to use 'rte_eth_devices[]' array directly.
But this array is marked as '@internal' and also it located in
the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
following disclaimer:

/**
* @file
*
* RTE Ethernet Device internal header.
*
* This header contains internal data types. But they are still part of the
* public API because they are used by inline functions in the published API.
*
* Applications should not use these directly.
*
*/

>From the other hand, test-pmd and some example apps in DPDK source
tree are using this array for various reasons.

So, is it OK to use this array directly or not?

Good question :)
Thanks for bringing this discussion.

As you said, it is public because of inline functions using it directly
for performance purpose. The DPDK API is bad for separating public and
internal stuff. And over time, there is not a lot of attention on trying
to not use internal symbols in applications.

In general we need to change the API, i.e. make 'rte_eth_devices' part
of a public API. Or change the test-pmd and example apps to stop
using it.

I agree we need to decide an option and make it clear.

We can try to make this variable private and add more public functions
to use it (I'm thinking at more iterators like sibling ones).
It would clarify the API.
It can be evaluated what is the real cost after compiler optimization
for Rx/Tx functions. It can also be evaluated to uninline functions.

On the other hand, we can wonder what is the real benefit of trying to
hide access to internal resources. Should we make all public?

In that case every change in any of such structures will be an ABI breakage.
Even now any change in rte_eth_dev is quite problematic because of that.
I think we better keep them private as much as possible and cleanup
our examples and testpmd code.
Konstantin

I Agree here, I have noticed a few places we allow direct access to internal 
data structures, which we need to restrict access by making them private with 
getter/setter functions or just getter/setter functions even if we can not make 
them private. At least with moving members and adding members we can state that 
it is not a ABI breakage as long as everyone uses the getter/setter functions. 
These functions could not be inline functions correct as that would still break 
API?


One more related question: Is it OK to access internal device
stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
This looks really dangerous. It's unclear why pointers like this
exposed to user.

It's a lot easier to expose pointers than doing a good API for all uses.
We need to question what is really dangerous and what we want to avoid?

Regards,
Keith

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Firewall/ Cloud Computing/ CRM Customer Base

2018-11-16 Thread Amanda Robinson
Hi,

Would you like to grow your business by acquiring data of new clients i.e., by 
acquiring C-Level, VP-Level, Directors & Managers Contacts of all Fortinet 
Users.

We can provide you with contacts from:

Firewall Softwares

CRM

Cloud Computing

Telecommunication

Cisco Firewall

Zoho

Oracle Cloud

VoIP

WatchGuard

MS Dynamics CRM

Amazon Web Services

Verizon

SonicWall

Bpm'online

Microsoft Azure

Shoretel

Juniper Firewall

SAP CRM

IBM Cloud

Mitel

Palo Alto Networks

Act! CRM

Cloud PBX

Brocade

Barracuda

Odoo

VMware

Juniper Networks

Check Point

SuiteCRM

Google Cloud Platform

AT

SiteLock

GoldMine CRM

OpenStack

Freshcaller

Cyberoam

SugarCRM

Kubernetes

Comcast

Sophos

Workbooks CRM

OpenNebula

CenturyLink

Zscaler

Sage CRM

SAP HANA

Slack

Comodo

Base CRM

Citrix

Jive


Information Fields we provide: - Name, Title, Email, Phone Numbers, Company 
Name, and Company Details like Physical Address, Web Address, Revenue Size, 
Employee Size, Software (in use) Industry and Sub-Industry.

Titles we provide -
C-Level

V-Levels

D-Levels

Others

CEO

Vice President

Director

President

CTO

VP Financial

Director VP Financial

Co-Founder/Founder

CFO

VP Business Development

Director Business Development

Partner

CIO

VP Marketing

Director Marketing

Chairman

COO

VP Sales

Director Sales

Vice Chairman

CCO

VP Operation

Director Operation

Executive Other

CKO

VP Technology

Director Technology

Board Member

CSO

VP Investment

Director Investment

Account Executive

CDO

VP Information

Director Software Development

Editor-In-Chief

CMO

VP IT

Director IT

Recruiter

CXO

VP Software Development

Director Product Marketing

Owner

CSO

VP Strategy

Director Software Development

AND MANY MORE TITLES


Please review and let me know if you are interested in purchasing the list, if 
you are not the right person regarding above email kindly forward the same to 
the relevant person/team for further assistance.

Warm Regards,
Amanda Robinson
Information Analyst

Database Reach - North America, EMEA, APAC and LATAM



 To OPT OUT, Please reply with "Remove" in the Subject Line


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap files.

2018-11-16 Thread Ben Pfaff
On Fri, Nov 16, 2018 at 12:42:12PM +0100, Timothy Redaelli wrote:
> On Thu, 15 Nov 2018 09:38:39 -0800
> Ben Pfaff  wrote:
> 
> > This broke the unit tests.
> > 
> > Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP 
> > files.")
> > Reported-by: Alin Gabriel Serdean 
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> Hi,
> my last nightly Fedora/CentOS tests [1] failed due to
> 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP
> files.").
> 
> I relaunched the CI with this patch and everything passed.
> 
> Tested-by: Timothy Redaelli 

Thanks Timothy and Alin.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] Windows: Fix broken kernel userspace communication

2018-11-16 Thread Alin Gabriel Serdean
Patch: 
https://github.com/openvswitch/ovs/commit/69c51582ff786a68fc325c1c50624715482bc460
broke Windows userpace - kernel communication.

On windows we create netlink sockets when the handlers are initiated and
reuse them.
This patch remaps the usage of the netlink socket pool.

Fixes:
https://github.com/openvswitch/ovs-issues/issues/164

Signed-off-by: Alin Gabriel Serdean 
Acked-by: Shashank Ram 
Tested-by: Shashank Ram 
Signed-off-by: Ben Pfaff 
Co-authored-by: Ben Pfaff 
---
v1 -> v2: Fold in nice incremental from Ben
RFC -> v1: Improve wording, change function name and add ifdef guard for them
---
 lib/dpif-netlink.c | 45 +
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 4ae132866..8d60e1d2e 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -246,6 +246,42 @@ static int dpif_netlink_port_query__(const struct 
dpif_netlink *dpif,
  odp_port_t port_no, const char *port_name,
  struct dpif_port *dpif_port);
 
+static int
+create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp)
+OVS_REQ_WRLOCK(dpif->upcall_lock)
+{
+#ifndef _WIN32
+return nl_sock_create(NETLINK_GENERIC, socksp);
+#else
+/* Pick netlink sockets to use in a round-robin fashion from each
+ * handler's pool of sockets. */
+struct dpif_handler *handler = >handlers[0];
+struct dpif_windows_vport_sock *sock_pool = handler->vport_sock_pool;
+size_t index = handler->last_used_pool_idx;
+
+/* A pool of sockets is allocated when the handler is initialized. */
+if (sock_pool == NULL) {
+*socksp = NULL;
+return EINVAL;
+}
+
+ovs_assert(index < VPORT_SOCK_POOL_SIZE);
+*socksp = sock_pool[index].nl_sock;
+ovs_assert(*socksp);
+index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1;
+handler->last_used_pool_idx = index;
+return 0;
+#endif
+}
+
+static void
+close_nl_sock(struct nl_sock *socksp)
+{
+#ifndef _WIN32
+nl_sock_destroy(socksp);
+#endif
+}
+
 static struct dpif_netlink *
 dpif_netlink_cast(const struct dpif *dpif)
 {
@@ -716,7 +752,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
char *name,
 int error = 0;
 
 if (dpif->handlers) {
-error = nl_sock_create(NETLINK_GENERIC, );
+error = create_nl_sock(dpif, );
 if (error) {
 return error;
 }
@@ -749,7 +785,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
char *name,
   dpif_name(>dpif), *port_nop);
 }
 
-nl_sock_destroy(socksp);
+close_nl_sock(socksp);
 goto exit;
 }
 
@@ -764,7 +800,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
char *name,
 request.dp_ifindex = dpif->dp_ifindex;
 request.port_no = *port_nop;
 dpif_netlink_vport_transact(, NULL, NULL);
-nl_sock_destroy(socksp);
+close_nl_sock(socksp);
 goto exit;
 }
 
@@ -2249,8 +2285,9 @@ dpif_netlink_refresh_channels(struct dpif_netlink *dpif, 
uint32_t n_handlers)
 if (port_no >= dpif->uc_array_size
 || !vport_get_pid(dpif, port_no, _pid)) {
 struct nl_sock *socksp;
+error = create_nl_sock(dpif, );
 
-if (nl_sock_create(NETLINK_GENERIC, )) {
+if (error) {
 goto error;
 }
 
-- 
2.16.1.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap files.

2018-11-16 Thread aserdean



> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, November 15, 2018 7:39 PM
> Către: d...@openvswitch.org
> Cc: Ben Pfaff ; Alin Gabriel Serdean 
> Subiect: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap
files.
> 
> This broke the unit tests.
> 
> Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP
> files.")
> Reported-by: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 
> ---
Acked-by: Alin Gabriel Serdean 
Tested-by: Alin Gabriel Serdean 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [RFC] OVN: add mac address only support to IPAM/MACAM

2018-11-16 Thread Lorenzo Bianconi
Add the capability to assign just the L2 address to IPAM/MACAM since
in the current implementation either subnet or ipv6_prefix are mandatory
to enable IPAM

Signed-off-by: Lorenzo Bianconi 
---
 ovn/northd/ovn-northd.c |  8 +++-
 ovn/ovn-nb.xml  |  5 +
 tests/ovn.at| 17 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4580cd705..9e547c7b2 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -414,6 +414,7 @@ struct ipam_info {
 unsigned long *allocated_ipv4s; /* A bitmap of allocated IPv4s */
 bool ipv6_prefix_set;
 struct in6_addr ipv6_prefix;
+bool mac_only;
 };
 
 /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
@@ -559,6 +560,10 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
 }
 
 if (!subnet_str) {
+if (!ipv6_prefix) {
+od->ipam_info.mac_only = smap_get_bool(>nbs->other_config,
+   "mac_only", false);
+}
 return;
 }
 
@@ -1382,7 +1387,8 @@ build_ipam(struct hmap *datapaths, struct hmap *ports)
 const struct nbrec_logical_switch_port *nbsp = od->nbs->ports[i];
 
 if (!od->ipam_info.allocated_ipv4s &&
-!od->ipam_info.ipv6_prefix_set) {
+!od->ipam_info.ipv6_prefix_set &&
+!od->ipam_info.mac_only) {
 if (nbsp->dynamic_addresses) {
 nbrec_logical_switch_port_set_dynamic_addresses(nbsp,
 NULL);
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 474b4f9a7..a5523ee18 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -267,6 +267,11 @@
   8230:5678::
 
   
+
+  
+Boolean value used to request to assing L2 address only if neigter 
subnet
+nor ipv6_prefix are specified
+  
 
 
 
diff --git a/tests/ovn.at b/tests/ovn.at
index ab32faa6b..15c96976a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5633,6 +5633,23 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p53 
dynamic_addresses], [0],
 ["00:11:22:a8:64:05 192.168.100.4"
 ])
 
+# request to assign mac only
+#
+ovn-nbctl ls-add sw7
+ovn-nbctl --wait=sb set Logical-Switch sw7 other_config:mac_only=true
+for n in $(seq 1 3); do
+ovn-nbctl --wait=sb lsp-add sw7 "p7$n" -- lsp-set-addresses "p7$n" dynamic
+done
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p71 dynamic_addresses], [0],
+["00:11:22:00:00:03"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p72 dynamic_addresses], [0],
+["00:11:22:00:00:04"
+])
+AT_CHECK([ovn-nbctl get Logical-Switch-Port p73 dynamic_addresses], [0],
+["00:11:22:00:00:05"
+])
+
 as ovn-sb
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
-- 
2.19.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] pcap: Fix reading regular old Ethernet pcap files.

2018-11-16 Thread Timothy Redaelli
On Thu, 15 Nov 2018 09:38:39 -0800
Ben Pfaff  wrote:

> This broke the unit tests.
> 
> Fixes: 597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP 
> files.")
> Reported-by: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 
> ---

Hi,
my last nightly Fedora/CentOS tests [1] failed due to
597177a283da ("pcap-file: Add support for Linux SLL formatted PCAP
files.").

I relaunched the CI with this patch and everything passed.

Tested-by: Timothy Redaelli 

[1] https://gitlab.com/drizzt/ovs/pipelines/36808576
[2] https://gitlab.com/drizzt/ovs/pipelines/36840304
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH dpdk-latest] netdev-dpdk: Fix returning the field of malloced struct.

2018-11-16 Thread Kevin Traynor
On 11/14/2018 01:41 PM, Ilya Maximets wrote:
> There is no sense returning the field because it never used.
> Function returns the pointer just to allow the caller to free it.
> Returning the pointer to the actual structure simplifies the code
> allowing not to explain why we're freeing the container of.
> 

Acked-by: Kevin Traynor 

> Signed-off-by: Ilya Maximets 
> ---
>  lib/netdev-dpdk.c | 15 ---
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 1480bf8d1..3e84a7fdf 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -4412,7 +4412,7 @@ struct action_rss_data {
>  uint16_t queue[0];
>  };
>  
> -static struct rte_flow_action_rss *
> +static struct action_rss_data *
>  add_flow_rss_action(struct flow_actions *actions,
>  struct netdev *netdev) {
>  int i;
> @@ -4439,7 +4439,7 @@ add_flow_rss_action(struct flow_actions *actions,
>  
>  add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, _data->conf);
>  
> -return _data->conf;
> +return rss_data;
>  }
>  
>  static int
> @@ -4649,7 +4649,7 @@ end_proto_check:
>  add_flow_pattern(, RTE_FLOW_ITEM_TYPE_END, NULL, NULL);
>  
>  struct rte_flow_action_mark mark;
> -struct rte_flow_action_rss *rss;
> +struct action_rss_data *rss;
>  
>  mark.id = info->flow_mark;
>  add_flow_action(, RTE_FLOW_ACTION_TYPE_MARK, );
> @@ -4664,14 +4664,7 @@ end_proto_check:
>  
>  ovs_mutex_unlock(>mutex);
>  
> -/*
> - * 'rss' points to a memory (struct rte_flow_action_rss) that is 
> contained
> - * in a bigger memory (struct action_rss_data) that was allocated in
> - * function add_flow_rss_actions(). The bigger memory holds additional
> - * space for the RSS queues. Given the 'rss' pointer we are freeing the
> - * bigger memory by using the CONTAINER_OF() macro.
> - */
> -free(CONTAINER_OF(rss, struct action_rss_data, conf));
> +free(rss);
>  if (!flow) {
>  VLOG_ERR("%s: rte flow creat error: %u : message : %s\n",
>   netdev_get_name(netdev), error.type, error.message);
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] OVN: add selected mac address to MACAM in update_dynamic_addresses

2018-11-16 Thread Lorenzo Bianconi
Add selected dynamic mac address to MACAM in update_dynamic_addresses
and not just in in ipam_add_port_addresses/ipam_insert_lsp_addresses
since the second approach can produce a duplicated L2 address in a
IPv6-only network if ipv6_prefix is provided after logical port creation.
The issue can be triggered with the following reproducer:

$ovn-nbctl ls-add sw0
$ovn-nbctl lsp-add sw0 sw0-port1
$ovn-nbctl lsp-set-addresses sw0-port1 "dynamic"
$ovn-nbctl lsp-add sw0 sw0-port2
$ovn-nbctl lsp-set-addresses sw0-port2 "dynamic"
$ovs-vsctl add-port br-int p1 -- \
set Interface p1 external_ids:iface-id=sw0-port1
$ovs-vsctl add-port br-int p2 -- \
set Interface p2 external_ids:iface-id=sw0-port2
[..]
$ovn-nbctl --wait=sb set Logical-switch sw0 \
other_config:ipv6_prefix="aef0::"

$ovn-nbctl list logical_switch_port
_uuid   : 1e0e2ed8-20c6-48dc-bfa8-d823e48c9f45
addresses   : [dynamic]
dhcpv4_options  : []
dhcpv6_options  : []
dynamic_addresses   : "0a:00:00:00:00:01 aef0::800:ff:fe00:1"
enabled : []
external_ids: {}
name: "sw0-port1"
options : {}
parent_name : []
port_security   : []
tag : []
tag_request : []
type: ""
up  : true

_uuid   : cfeab7fb-e20b-41f1-974c-f99e0b5293d7
addresses   : [dynamic]
dhcpv4_options  : []
dhcpv6_options  : []
dynamic_addresses   : "0a:00:00:00:00:01 aef0::800:ff:fe00:1"
enabled : []
external_ids: {}
name: "sw0-port2"
options : {}
parent_name : []
port_security   : []
tag : []
tag_request : []
type: ""
up  : true

Fixes: c814545b43ac ("OVN: configure L2 address according to the used IP
address")

Signed-off-by: Lorenzo Bianconi 
---
 ovn/northd/ovn-northd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 58bef7de5..4580cd705 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1341,6 +1341,8 @@ update_dynamic_addresses(struct dynamic_address_update 
*update)
 
 struct ds new_addr = DS_EMPTY_INITIALIZER;
 ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+ipam_insert_mac(, true);
+
 if (ip4) {
 ipam_insert_ip(update->od, ntohl(ip4));
 ds_put_format(_addr, " "IP_FMT, IP_ARGS(ip4));
-- 
2.19.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] OVN multicast enhancements

2018-11-16 Thread Ankur Sharma
Hi Mark,

Thanks for the write up.
Initial proposal looks to be a good starting point.

Please find my feedback inline.


Regards,
Ankur


From: ovs-dev-boun...@openvswitch.org  on 
behalf of Mark Michelson 
Sent: Thursday, November 15, 2018 2:20 PM
To: ovs-dev@openvswitch.org
Subject: [ovs-dev] OVN multicast enhancements

Hi everyone,

In today's IRC meeting I brought up the fact I was enhancing multicast
support in OVN, and I was asked to expand on this a bit. So here are
details about what all I am working on.

Right now, OVN logical switches treat all multicast destinations as if
they were broadcast. That is, the packet gets sent to all ports on the
switch. OVN logical routers block all traffic destined to a multicast
address.

My goal is for there to be more intelligence. IP traffic sent to
multicast addresses should be sent to members of that multicast group
when possible.

My work consists of the following phases:

Phase 1: Northbound changes; multicast on a single logical switch port.

For this part, we create a new northbound table called Multicast_Group.
It consists of:
* A name
* A multicast IP address
* A list of logical switch ports
The general idea is that traffic sent to the IP address should reach all
logical switch ports listed.

[ANKUR]:
a. I am assuming this table is per logical switch.

b. How will you discover the port to multicast group mapping?
I am of the opinion that instead of having an out of band mechanism, OVN can 
leverage on igmp snopping in OVS and populate this multicast group to port 
mapping.
We can also enhance as ovn-controller to do local igmp snooping and populate 
this table.

c. Is it really multicast or multi unicast?
i.e for intra chassis logical ports, will ovn-controller generate on packet for 
each endpoint?
Similarly, for endpoints in a remote chassis, how many packets will source 
chassis generate?


In practice, each northbound multicast group will result in southbound
multicast groups being added to the datapaths that the logical switch
ports reside on. Logical flows are introduced on the logical switch
ingress pipelines to output packets that are destined for the multicast
IP address and the corresponding derived multicast ethernet address [1]
to the constituent ports.
[ANKUR]:
To be honest, i am not clear with the use case of multicast_group table in 
southbound DB.
Can you elaborate on it please? It will help me with better understanding of 
the proposal.


In practice, this set of changes only works if all ports in the
multicast group are on the same logical switch.

I already have this implemented and have a test written for the
testsuite that passes. You can see what I have at
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_putnopvut_ovs_tree_multicast-2Dimprovements=DwICAg=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=FzqJqkEOtSxSyDtTFtQYFLaAR8OPuTmCm85UStogNeI=kC2cxfPhTHmGGJ263RuiYNeBHuBVMBeBxjpjHCmrzlM=
[https://avatars1.githubusercontent.com/u/1328590?s=400=4]

putnopvut/ovs
urldefense.proofpoint.com
Open vSwitch. Contribute to putnopvut/ovs development by creating an account on 
GitHub.




Phase 2: Add multicast routing support

This expands on the work in phase 1 by creating southbound multicast
groups on logical router datapaths for router ports connected to logical
switches with multicast groups on them. This way, a multicast group
distributed across multiple logical switches can have the traffic routed
as desired.

This also requires adding new logical flows for the router pipeline to
ensure that traffic bound to known multicast IPs is sent where we expect.

I'm currently working on this phase. I have a test written and it is not
passing.

[ANKUR]:
Looks like this proposal is only for E-W traffic. For N-S we will have to 
advertise NATed IPs as multicast group member to uplink router.


Phase 3: Expand to IPv6 support.

The previous phases are very IPv4-centric. This phase expands on what we
have already by making it work with IPv6 as well. Mostly, this means
removing IPv4-centric idioms and installing IPv6 flows in parallel with
the IPv4 flows.


With those three phases complete, I think I'll have a decent patchset to
submit for approval. There are other enhancements that can be
implemented as well later on:

* IGMP/MLD snooping in ovn-controller.
* Installation of rules for well-known multicast addresses with semantic
significance.


Re: [ovs-dev] [PATCH v3] netdev: Retry getting interfaces on inconsistent dumps from kernel

2018-11-16 Thread Daniel Alvarez Sanchez
I forgot about this, sorry. Just sent the patch with the comment.
Thanks a lot,
Daniel
On Sat, Aug 18, 2018 at 5:41 PM Ben Pfaff  wrote:
>
> On August 18, 2018 8:18:51 AM PDT, Daniel Alvarez Sanchez 
>  wrote:
>>
>> Thanks a lot Ben.
>> It was fixed in glibc 2.28. Shall I send a patch to add the comment in the 
>> code? I think it's a good idea so that we can remove the workaround 
>> eventually. Not sure about sending a patch for just a comment though :)
>>
>> On Wed, Aug 15, 2018 at 10:40 PM Ben Pfaff  wrote:
>>>
>>> On Mon, Aug 13, 2018 at 05:39:07PM -0700, Ben Pfaff wrote:
>>> > On Mon, Aug 13, 2018 at 02:07:45PM +0200, Daniel Alvarez wrote:
>>> > > This patch in glibc [0] is fixing a bug where we may be getting
>>> > > inconsistent dumps from the kernel when listing interfaces due to
>>> > > a race condition.
>>> > >
>>> > > This could happen if we try to retrieve them while interfaces are
>>> > > being added/removed from the system at the same time.
>>> > > For systems running against old glibc versions, this patch is retrying
>>> > > the operation up to 3 times and then proceeding by logging a
>>> > > warning.
>>> > >
>>> > > Note that 3 times should be enough to not delay the operation much
>>> > > and since it's unlikely that we hit the race condition 3 times in
>>> > > a row. Still, if this happened, this patch is not changing the
>>> > > current behavior.
>>> > >
>>> > > [0] 
>>> > > https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c1f86a33ca32e26a9d6e29fc961e5ecb5e2e5eb4
>>> > >
>>> > > Signed-off-by: Daniel Alvarez 
>>> > > Co-authored-by: Jiri Benc 
>>> >
>>> > Thanks for the patch.
>>> >
>>> > As a co-author, Jiri also needs to sign off.
>>> >
>>> > Acked-by: Ben Pfaff 
>>>
>>> I see that Jiri signed off earlier and it just didn't get propagated.
>>>
>>> Thanks, I applied this to master and branch-2.10.
>
>
> The comment would be useful, so if you have time for it please send a patch.
>
> Thanks,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] netdev: Add comment to allow removing a workaround in the future

2018-11-16 Thread Daniel Alvarez
This patch [0] in glibc fixes an issue which is right now workarounded
in OVS by [1]. I'm adding a comment to indicate that from glibc 2.28
and beyond, the workaround is not needed so that we can eventually
remove it.

[0] 
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c1f86a33ca32e26a9d6e29fc961e5ecb5e2e5eb4
[1] 
https://github.com/openvswitch/ovs/commit/3434d306866d825084d2d186d1f8dd98662ff650

Signed-off-by: Daniel Alvarez 
---
 lib/netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netdev.c b/lib/netdev.c
index 84874408a..45b50f26c 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2063,7 +2063,8 @@ retry:
  * address addition which may cause one of the returned
  * ifa_name values to be NULL. In such case, we know that we've
  * got an inconsistent dump. Retry but beware of an endless
- * loop. */
+ * loop. From glibc 2.28 and beyond, this workaround is not
+ * needed and should be eventually removed. */
 freeifaddrs(if_addr_list);
 goto retry;
 } else {
--
2.17.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] OVS-DPDK public meeting

2018-11-16 Thread Ilya Maximets
> 
> On 15 Nov 2018, at 15:18, Kevin Traynor wrote:
> 
>> Next meeting 12th December
>> (May cancel if no agenda items as DPDK/OVS confs the previous week)
>>
>> Minutes for 14th November.
>> Attendees: Pieter, Asaf, Timothy, Ophir, Ian, Thomas, David, Matteo,
>> Johann, Aaron, Tiago, Kevin.
>>
>> ===
>> GENERAL
>> ===
>> - Ian is one of the core committers :-)
>> -- He will still use his private branch for validation
>> -- Patches will be applied directly into main tree
>> -- i.e. no more pull requests
>>
>> 
>> PERFORMANCE/FEATURES
>> 
>> - DPDK 18.08/18.11 update (Kevin)
>> -- sent patches for update to 18.11
>> -- Ian found an issue with CRC strip, to be fixed
>> -- Ophir also sent a patch prior to meeting
>> -- Thomas M, explained some of the new features of hotplug
>> -- Discussed integration of changes and if it should be broken up 
>> into:
>> -- 1. Minimal update to use 18.11
>> -- 2. Extend OVS to take advantage of new features in 18.11
>> -- Discussion to be continued on ML
>>
>> - CI lab (Ophir/Aaron/Ian)
>> -- Not much changed
>> -- Testing done in VM
>> -- Mlx to check about Hw for Aaron
>>
>> - 17.11.4 (Eelco/Aaron)
>> -- Slow start up issue reported because of some patches in 17.11.4
>> -- Alejandro sent patch for DPDK 17.11 branch
>> -- https://mails.dpdk.org/archives/dev/2018-November/118770.html
>> -- Eelco will add some more details
> 
> This issue got introduced due to commit 293c0c4b9. This change will try 
> to map the huge page memory we need starting at 0x1. If it fails 
> the hint, it will retry increasing the address, until it gets the 
> mapping at the requested address. This to minimise the chance to map it 
> outside the HWs DMA address space.
> 
> However just before doing this DPDK also mmaps each individual huge page 
> in the system. It starts low, but it will collide with the 0x1 
> address.
> 
> When this happens there a quite some retries until we find the mmaps we 
> want. So the more memory in huge pages, the more time it takes. It 
> depends on the number of huge pages in the system, not the memory 
> configured for OVS.
> 
> FYI this mapping on a 32G huge page system:
> 4000-8000 rw-s  00:23 206083 
> /dev/hugepages/rtemap_1
> 8000-c000 rw-s  00:23 206084 
> /dev/hugepages/rtemap_2
> c000-1 rw-s  00:23 206085
> /dev/hugepages/rtemap_3
> 1-14000 rw-s  00:23 206086   
> /dev/hugepages/rtemap_4
> 14000-18000 rw-s  00:23 206087   
> /dev/hugepages/rtemap_5
> 18000-1c000 rw-s  00:23 206088   
> /dev/hugepages/rtemap_6
> 1c000-2 rw-s  00:23 206089   
> /dev/hugepages/rtemap_7
> 2-24000 rw-s  00:23 206090   
> /dev/hugepages/rtemap_8
> 24000-28000 rw-s  00:23 206091   
> /dev/hugepages/rtemap_9
> 28000-2c000 rw-s  00:23 206092   
> /dev/hugepages/rtemap_10
> 2c000-3 rw-s  00:23 206093   
> /dev/hugepages/rtemap_11
> 3-34000 rw-s  00:23 206094   
> /dev/hugepages/rtemap_12
> 34000-38000 rw-s  00:23 206095   
> /dev/hugepages/rtemap_13
> 38000-3c000 rw-s  00:23 206096   
> /dev/hugepages/rtemap_14
> 3c000-4 rw-s  00:23 206097   
> /dev/hugepages/rtemap_15
> 4-44000 rw-s  00:23 206098   
> /dev/hugepages/rtemap_16
> 44000-48000 rw-s  00:23 206099   
> /dev/hugepages/rtemap_17
> 48000-4c000 rw-s  00:23 206100   
> /dev/hugepages/rtemap_18
> 4c000-5 rw-s  00:23 206101   
> /dev/hugepages/rtemap_19
> 5-54000 rw-s  00:23 206102   
> /dev/hugepages/rtemap_20
> 54000-58000 rw-s  00:23 206103   
> /dev/hugepages/rtemap_21
> 58000-5c000 rw-s  00:23 206104   
> /dev/hugepages/rtemap_22
> 5c000-6 rw-s  00:23 206105   
> /dev/hugepages/rtemap_23
> 6-64000 rw-s  00:23 206106   
> /dev/hugepages/rtemap_24
> 64000-68000 rw-s  00:23 206107   
> /dev/hugepages/rtemap_25
> 68000-6c000 rw-s  00:23 206108   
> /dev/hugepages/rtemap_26
> 6c000-7 rw-s  00:23 206109   
> /dev/hugepages/rtemap_27
> 

Re: [ovs-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Ananyev, Konstantin
Hi everyone,

> 
> Hi,
> 
> 16/11/2018 09:42, Ilya Maximets:
> > Hi,
> > While discussing the ways to enable DPDK 18.11 new features in OVS
> > there was suggestions to use 'rte_eth_devices[]' array directly.
> > But this array is marked as '@internal' and also it located in
> > the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
> > following disclaimer:
> >
> > /**
> >  * @file
> >  *
> >  * RTE Ethernet Device internal header.
> >  *
> >  * This header contains internal data types. But they are still part of the
> >  * public API because they are used by inline functions in the published 
> > API.
> >  *
> >  * Applications should not use these directly.
> >  *
> >  */
> >
> > From the other hand, test-pmd and some example apps in DPDK source
> > tree are using this array for various reasons.
> >
> > So, is it OK to use this array directly or not?
> 
> Good question :)
> Thanks for bringing this discussion.
> 
> As you said, it is public because of inline functions using it directly
> for performance purpose. The DPDK API is bad for separating public and
> internal stuff. And over time, there is not a lot of attention on trying
> to not use internal symbols in applications.
> 
> > In general we need to change the API, i.e. make 'rte_eth_devices' part
> > of a public API. Or change the test-pmd and example apps to stop
> > using it.
> 
> I agree we need to decide an option and make it clear.
> 
> We can try to make this variable private and add more public functions
> to use it (I'm thinking at more iterators like sibling ones).
> It would clarify the API.
> It can be evaluated what is the real cost after compiler optimization
> for Rx/Tx functions. It can also be evaluated to uninline functions.
> 
> On the other hand, we can wonder what is the real benefit of trying to
> hide access to internal resources. Should we make all public?

In that case every change in any of such structures will be an ABI breakage.
Even now any change in rte_eth_dev is quite problematic because of that.
I think we better keep them private as much as possible and cleanup
our examples and testpmd code.
Konstantin

> 
> > One more related question: Is it OK to access internal device
> > stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
> > This looks really dangerous. It's unclear why pointers like this
> > exposed to user.
> 
> It's a lot easier to expose pointers than doing a good API for all uses.
> We need to question what is really dangerous and what we want to avoid?
> 
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Thomas Monjalon
Hi,

16/11/2018 09:42, Ilya Maximets:
> Hi,
> While discussing the ways to enable DPDK 18.11 new features in OVS
> there was suggestions to use 'rte_eth_devices[]' array directly.
> But this array is marked as '@internal' and also it located in
> the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
> following disclaimer:
> 
> /**
>  * @file
>  *
>  * RTE Ethernet Device internal header.
>  *
>  * This header contains internal data types. But they are still part of the
>  * public API because they are used by inline functions in the published API.
>  *
>  * Applications should not use these directly.
>  *
>  */
> 
> From the other hand, test-pmd and some example apps in DPDK source
> tree are using this array for various reasons.
> 
> So, is it OK to use this array directly or not?

Good question :)
Thanks for bringing this discussion.

As you said, it is public because of inline functions using it directly
for performance purpose. The DPDK API is bad for separating public and
internal stuff. And over time, there is not a lot of attention on trying
to not use internal symbols in applications.

> In general we need to change the API, i.e. make 'rte_eth_devices' part
> of a public API. Or change the test-pmd and example apps to stop
> using it.

I agree we need to decide an option and make it clear.

We can try to make this variable private and add more public functions
to use it (I'm thinking at more iterators like sibling ones).
It would clarify the API.
It can be evaluated what is the real cost after compiler optimization
for Rx/Tx functions. It can also be evaluated to uninline functions.

On the other hand, we can wonder what is the real benefit of trying to
hide access to internal resources. Should we make all public?

> One more related question: Is it OK to access internal device
> stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
> This looks really dangerous. It's unclear why pointers like this
> exposed to user.

It's a lot easier to expose pointers than doing a good API for all uses.
We need to question what is really dangerous and what we want to avoid?



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Ferruh Yigit
On 11/16/2018 8:42 AM, Ilya Maximets wrote:
> Hi,
> While discussing the ways to enable DPDK 18.11 new features in OVS
> there was suggestions to use 'rte_eth_devices[]' array directly.
> But this array is marked as '@internal' and also it located in
> the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
> following disclaimer:
> 
> /**
>  * @file
>  *
>  * RTE Ethernet Device internal header.
>  *
>  * This header contains internal data types. But they are still part of the
>  * public API because they are used by inline functions in the published API.
>  *
>  * Applications should not use these directly.
>  *
>  */
> 
> From the other hand, test-pmd and some example apps in DPDK source
> tree are using this array for various reasons.
> 
> So, is it OK to use this array directly or not?
> 
> In general we need to change the API, i.e. make 'rte_eth_devices' part
> of a public API. Or change the test-pmd and example apps to stop
> using it.
> 
> One more related question: Is it OK to access internal device
> stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
> This looks really dangerous. It's unclear why pointers like this
> exposed to user.
> 
> Thoughts?

Agreed that applications shouldn't access 'rte_eth_devices' directly, they
should use handler (port_id) to access devices. Only drivers are allowed to
access these data structure directly.

Because of inline functions we are not able to completely hide these from
applications, and preferring inline functions for performance. Moving these data
structures to their own header and marking as internal was to clarify the
intended usage for them.

I support to clean testpmd and sample applications to prevent using internal
data structures.

> 
> Best regards, Ilya Maximets.
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Direct using of 'rte_eth_devices' in DPDK apps.

2018-11-16 Thread Ilya Maximets
Hi,
While discussing the ways to enable DPDK 18.11 new features in OVS
there was suggestions to use 'rte_eth_devices[]' array directly.
But this array is marked as '@internal' and also it located in
the internal header 'lib/librte_ethdev/rte_ethdev_core.h' with the
following disclaimer:

/**
 * @file
 *
 * RTE Ethernet Device internal header.
 *
 * This header contains internal data types. But they are still part of the
 * public API because they are used by inline functions in the published API.
 *
 * Applications should not use these directly.
 *
 */

>From the other hand, test-pmd and some example apps in DPDK source
tree are using this array for various reasons.

So, is it OK to use this array directly or not?

In general we need to change the API, i.e. make 'rte_eth_devices' part
of a public API. Or change the test-pmd and example apps to stop
using it.

One more related question: Is it OK to access internal device
stuff using 'device' pointer obtained by 'rte_eth_dev_info'?
This looks really dangerous. It's unclear why pointers like this
exposed to user.

Thoughts?

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [RFC dpdk-latest 2/2] netdev-dpdk: Replace rte_eth_dev_attach/detach.

2018-11-16 Thread Ilya Maximets
On 15.11.2018 20:26, Kevin Traynor wrote:
> On 11/15/2018 04:38 PM, Ophir Munk wrote:
>> Hi Kevin,
>> Thanks for this RFC. Please find comments inline.
>>
>>> -Original Message-
>>> From: Kevin Traynor [mailto:ktray...@redhat.com]
>>> Sent: Thursday, November 08, 2018 8:37 PM
>>> To: d...@openvswitch.org; Ophir Munk ;
>>> ian.sto...@intel.com; i.maxim...@samsung.com
>>> Cc: Kevin Traynor 
>>> Subject: [RFC dpdk-latest 2/2] netdev-dpdk: Replace
>>> rte_eth_dev_attach/detach.
>>>
>>> rte_eth_dev_attach/detach have been removed from DPDK 18.11. Replace
>>> them with rte_dev_probe/remove.
>>>
>>
>> I have submitted a patch on the same issue, see [1]. 
>> Please suggest how to unify our patches.
>>
> 
> Hi Ophir,
> 
> I looked through your patch and it is trying to do two things:
> 1. update OVS to use DPDK 18.11
> 2. enable additional functionality/representors etc in OVS based on
> using DPDK 18.11
> 
> I don't think that needs to be all in one patch and there isn't really
> any throw away work in doing 1. on it's own. My suggestion is that we
> proceed with 1. through the patches I sent, and then (or in parallel)
> you can send patches to cover 2.
> 
> What do people think?

+1. Sounds good.


> thanks,
> Kevin.
> 
>>> Signed-off-by: Kevin Traynor 
>>> ---
>>>  lilib/librte_ethdev/rte_ethdev_core.hb/netdev-dpdk.c | 29 
>>> +
>>>  1 file changed, 17 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>> 10c4879a1..190d50007 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -1351,5 +1351,5 @@ netdev_dpdk_destruct(struct netdev *netdev)  {
>>>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>> -char devname[RTE_ETH_NAME_MAX_LEN];
>>> +struct rte_eth_dev_info dev_info;
>>>
>>>  ovs_mutex_lock(_mutex);
>>> @@ -1360,8 +1360,9 @@ netdev_dpdk_destruct(struct netdev *netdev)
>>>  if (dev->attached) {
>>>  rte_eth_dev_close(dev->port_id);
>>> -if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
>>> +rte_eth_dev_info_get(dev->port_id, _info);
>>
>> I suggest having a direct access to the device (rather than accessing 
>> dev_info).
>> rte_eth_devices[dev->port_id].device;

I don't think that we can use 'rte_eth_devices' directly because it
marked as @internal in dpdk code. Also it's located in file
lib/librte_ethdev/rte_ethdev_core.h, which has following disclaimer:

/**
 * @file
 *
 * RTE Ethernet Device internal header.
 *
 * This header contains internal data types. But they are still part of the
 * public API because they are used by inline functions in the published API.
 *
 * Applications should not use these directly.
 *
 */

I'd like to raise this issue in DPDK community, because test-pmd and some
example apps are using this array directly.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev