Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugins.c Internal error? #759

Closed
woytowg opened this issue Apr 22, 2019 · 26 comments
Closed

plugins.c Internal error? #759

woytowg opened this issue Apr 22, 2019 · 26 comments

Comments

@woytowg
Copy link

woytowg commented Apr 22, 2019

Here's the original error

Apr 22 17:08:59 comx178f37 sysrepod[613]: [ERR] (dm_ly_log_cb:1617) libyang error: Internal error (../libyang/libyang/src/plugins.c:654).

I added a debug log message to the plugins.c file to get more details, which are shown here

Apr 22 17:08:59 comx178f37 sysrepod[613]: [ERR] (dm_ly_log_cb:1617) libyang error: lytype_find failed (ietf-inet-types:2013-07-15:ipv4-address).

It is having a problem with the ipv4-address type from the ietf-inet-types yang file. I tried explicitly installing the ietf-inet-types.yang file with the same results.

I just did a pull to the latest version 798f011 and started seeing this problem, I was using 3ae919e previously without these errors.

It seems like everything is working, but just getting a bunch of errors

@michalvasko
Copy link
Member

Hi,
could you please provide steps to reproduce the error? Thanks.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented Apr 23, 2019

Not quite sure how to reproduce, It happens in part of a large system, I can debug it, here is a stack trace, maybe that helps?

#0 lytype_free (type=0x558fb464a4b0, value=..., value_str=) at libyang/src/plugins.c:654
#1 0x00007f7b93ba51ea in _lyd_free_node (node=0x558fb4742940) at libyang/src/tree_data.c:6135
#2 0x00007f7b93ba560f in lyd_free_withsiblings_r (first=) at libyang/src/tree_data.c:6184
#3 0x00007f7b93ba5633 in lyd_free_withsiblings_r (first=) at libyang/src/tree_data.c:6182
#4 0x00007f7b93ba5633 in lyd_free_withsiblings_r (first=) at libyang/src/tree_data.c:6182
#5 0x00007f7b93ba5633 in lyd_free_withsiblings_r (first=) at libyang/src/tree_data.c:6182
#6 0x00007f7b93ba5633 in lyd_free_withsiblings_r (first=) at libyang/src/tree_data.c:6182
#7 0x00007f7b95e5cdf9 in dm_remove_added_data_trees (session=0x558fb46e8540, data_info=0x558fb46f64d0) at sysrepo/src/data_manager.c:2203
#8 0x00007f7b95e669d3 in dm_validate_data_info (dm_ctx=0x558fb3b6def0, session=0x558fb46e8540, info=0x558fb46f64d0) at sysrepo/src/data_manager.c:2881
#9 0x00007f7b95e6921a in dm_get_data_info_internal (dm_ctx=0x558fb3b6def0, dm_session_ctx=0x558fb46e8540, module_name=0x558fb471a360 "ietf-routing", skip_validation=false, must_be_freed=0x0, info=0x7f7b918a7af8)
at sysrepo/src/data_manager.c:3019
#10 0x00007f7b95e69b5f in dm_get_data_info (dm_ctx=0x558fb3b6def0, dm_session_ctx=0x558fb46e8540, module_name=0x558fb471a360 "ietf-routing", info=0x7f7b918a7af8) at sysrepo/src/data_manager.c:3054
#11 0x00007f7b95e31e02 in rp_dt_delete_item (dm_ctx=0x558fb3b6def0, session=0x558fb46e8540, xpath=0x558fb47cd710 "/ietf-routing:routing-state", options=SR_EDIT_DEFAULT, is_state=true) at sysrepo/src/rp_dt_edit.c:194
#12 0x00007f7b95e21c34 in rp_dt_remove_loaded_state_data (rp_ctx=0x558fb3b2f520, rp_session=0x558fb46e93d0) at sysrepo/src/rp_dt_get.c:1171
#13 0x00007f7b95e222d1 in rp_dt_prepare_data (rp_ctx=0x558fb3b2f520, rp_session=0x558fb46e93d0, xpath=0x558fb479c890 "/ietf-yang-library://.", api_variant=SR_API_VALUES, tree_depth_limit=0, data_tree=0x7f7b918a7c88)
at sysrepo/src/rp_dt_get.c:1196
#14 0x00007f7b95e25b1d in rp_dt_get_values_wrapper_with_opts (rp_ctx=0x558fb3b2f520, rp_session=0x558fb46e93d0, get_items_ctx=0x558fb46e9430, sr_mem=0x0, xpath=0x558fb479c890 "/ietf-yang-library:
//.", offset=0, limit=100, values=0x7f7b918a7d08, count=0x7f7b918a7d10)
at sysrepo/src/rp_dt_get.c:1360
#15 0x00007f7b95dd93a2 in rp_get_items_req_process (rp_ctx=0x558fb3b2f520, session=0x558fb46e93d0, msg=0x558fb47d1f10, skip_msg_cleanup=0x7f7b918a7e13) at sysrepo/src/request_processor.c:822
#16 0x00007f7b95e031cc in rp_req_dispatch (rp_ctx=0x558fb3b2f520, session=0x558fb46e93d0, msg=0x558fb47d1f10, skip_msg_cleanup=0x7f7b918a7e13) at sysrepo/src/request_processor.c:3457
#17 0x00007f7b95e05315 in rp_msg_dispatch (rp_ctx=0x558fb3b2f520, session=0x558fb46e93d0, msg=0x558fb47d1f10) at sysrepo/src/request_processor.c:3668
#18 0x00007f7b95e06367 in rp_worker_thread_execute (rp_ctx_p=0x558fb3b2f520) at sysrepo/src/request_processor.c:3769
#19 0x00007f7b95a126db in start_thread (arg=0x7f7b918a8700) at pthread_create.c:463
~

@michalvasko
Copy link
Member

Hi,
try to pull the latest version and retry your use-case, whether the problem was not fixed (in 1.0.9 there could be a fix for this). If not then we can try to find out more.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented Apr 24, 2019

Just tried c39aef6 (1.0.13) with the same result, still getting internal errors on the ipv4-address.

I'm pretty sure the inet:ipv4-address causing problems is the "*leaf next-hop-address" in
the "grouping next-hop-content" in ietf-routing.yang model

I don't understand your comment above regarding 1.0.9?

@michalvasko
Copy link
Member

Hi,
I just wanted you to try the recent version and you did. I tried to find ietf-routing revision that would have leaf next-hop-address in the grouping next-hop-content but failed (briefly found 3 other revisions) so can you please attach it? It would also be helpful if you could provide some data for it that I can use to reproduce the problem. Otherwise I will try to write my own.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented Apr 24, 2019

module ietf-routing {
  namespace "urn:ietf:params:xml:ns:yang:ietf-routing";
  prefix "rt";

  import ietf-inet-types {
    prefix "inet";
  }

  import ietf-yang-types {
    prefix "yang";
  }

  import ietf-interfaces {
    prefix "if";
  }

  organization
    "IETF NETMOD (NETCONF Data Modeling Language) Working Group";
  contact
    "WG Web:   <http://tools.ietf.org/wg/netmod/>
     WG List:  <mailto:[email protected]>

     WG Chair: Lou Berger
               <mailto:[email protected]>

     WG Chair: Juergen Schoenwaelder
               <mailto:[email protected]>

     WG Chair: Kent Watsen
               <mailto:[email protected]>

     Editor:   Ladislav Lhotka
               <mailto:[email protected]>

     Editor:   Acee Lindem
               <mailto:[email protected]>";

  description
    "This YANG module defines essential components for the management
     of a routing subsystem.

     Copyright (c) 2016 IETF Trust and the persons identified as
     authors of the code. All rights reserved.

     Redistribution and use in source and binary forms, with or
     without modification, is permitted pursuant to, and subject to
     the license terms contained in, the Simplified BSD License set
     forth in Section 4.c of the IETF Trust's Legal Provisions
     Relating to IETF Documents
     (http://trustee.ietf.org/license-info).

     The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL', 'SHALL
     NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED', 'MAY', and
     'OPTIONAL' in the module text are to be interpreted as described
     in RFC 2119 (http://tools.ietf.org/html/rfc2119).

     This version of this YANG module is part of RFC XXXX
     (http://tools.ietf.org/html/rfcXXXX); see the RFC itself for
     full legal notices.";

  revision 2016-03-09 {
    description
      "Initial revision.";
    reference
      "RFC XXXX: A YANG Data Model for Routing Management";
  }
  /* Features */

  feature multiple-ribs {
    description
      "This feature indicates that the server supports user-defined
       RIBs.

       Servers that do not advertise this feature SHOULD provide
       exactly one system-controlled RIB per supported address family
       and make them also the default RIBs. These RIBs then appear as
       entries of the list /routing-state/ribs/rib.";
  }

  feature router-id {
    description
      "This feature indicates that the server supports configuration
       of an explicit 32-bit router ID that is used by some routing
       protocols.

       Servers that do not advertise this feature set a router ID
       algorithmically, usually to one of configured IPv4 addresses.
       However, this algorithm is implementation-specific.";
  }

  /* Identities */

  identity address-family {
    description
      "Base identity from which identities describing address
       families are derived.";
  }

  identity ipv4 {
    base address-family;
    description
      "This identity represents IPv4 address family.";
  }

  identity ipv6 {
    base address-family;
    description
      "This identity represents IPv6 address family.";
  }

  identity routing-protocol {
    description
      "Base identity from which routing protocol identities are
       derived.";
  }

  identity direct {
    base routing-protocol;
    description
      "Routing pseudo-protocol that provides routes to directly
       connected networks.";
  }

  identity static {
    base routing-protocol;
    description
      "Static routing pseudo-protocol.";
  }

  /* Type Definitions */

  typedef route-preference {
    type uint32;
    description
      "This type is used for route preferences.";
  }

  /* Groupings */

  grouping address-family {
    description
      "This grouping provides a leaf identifying an address
       family.";
    leaf address-family {
      type identityref {
        base address-family;
      }
      mandatory "true";
      description
        "Address family.";
    }
  }

  grouping router-id {
    description
      "This grouping provides router ID.";
    leaf router-id {
      type yang:dotted-quad;
      description
        "A 32-bit number in the form of a dotted quad that is used by
         some routing protocols identifying a router.";
      reference
        "RFC 2328: OSPF Version 2.";
    }
  }

  grouping special-next-hop {
    description
      "This grouping provides a leaf with an enumeration of special
       next-hops.";
    leaf special-next-hop {
      type enumeration {
        enum blackhole {
          description
            "Silently discard the packet.";
        }
        enum unreachable {
          description
            "Discard the packet and notify the sender with an error
             message indicating that the destination host is
             unreachable.";
        }
        enum prohibit {
          description
            "Discard the packet and notify the sender with an error
             message indicating that the communication is
             administratively prohibited.";
        }
        enum receive {
          description
            "The packet will be received by the local system.";
        }
      }
      description
        "Special next-hop options.";
    }
  }

  grouping next-hop-content {
    description
      "Generic parameters of next-hops in static routes.";
    choice next-hop-options {
      mandatory "true";
      description
        "Options for next-hops in static routes.

         Modules for address families MUST augment this choice with
         the 'next-hop-address' case, which is a leaf containing a
         gateway address of that address family.

         It is expected that further cases will be added through
         augments from other modules, e.g., for Equal-Cost Multipath
         routing (ECMP).";
      leaf outgoing-interface {
        type if:interface-ref;
        description
          "Name of the outgoing interface.";
      }
      case special-next-hop {
        uses special-next-hop;
      }
      leaf next-hop-address {
        type inet:ipv4-address;
        description
          "IPv4 address of the next-hop.";
      }
    }
  }

  grouping next-hop-state-content {
    description
      "Generic parameters of next-hops in state data.";
    choice next-hop-options {
      mandatory "true";
      description
        "Options for next-hops in state data.

         Modules for address families MUST augment this choice with
         the 'next-hop-address' case, which is a leaf containing a
         gateway address of that address family.

         It is expected that further cases will be added through
         augments from other modules, e.g., for ECMP or recursive
         next-hops.";
      leaf outgoing-interface {
        type if:interface-state-ref;
        description
          "Name of the outgoing interface.";
      }
      case special-next-hop {
        uses special-next-hop;
      }
    }
  }

  grouping route-metadata {
    description
      "Common route metadata.";
    leaf source-protocol {
      type identityref {
        base routing-protocol;
      }
      mandatory "true";
      description
        "Type of the routing protocol from which the route
         originated.";
    }
    leaf active {
      type empty;
      description
        "Presence of this leaf indicates that the route is preferred
         among all routes in the same RIB that have the same
         destination prefix.";
    }
    leaf last-updated {
      type yang:date-and-time;
      description
        "Time stamp of the last modification of the route. If the
         route was never modified, it is the time when the route was
         inserted into the RIB.";
    }
  }

  /* State data */

  container routing-state {
    config "false";
    description
      "State data of the routing subsystem.";
    uses router-id {
      description
        "Global router ID.

         It may be either configured or assigned algorithmically by
         the implementation.";
    }
    container interfaces {
      description
        "Network layer interfaces used for routing.";
      leaf-list interface {
        type if:interface-state-ref;
        description
          "Each entry is a reference to the name of a configured
           network layer interface.";
      }
    }
    container routing-protocols {
      description
        "Container for the list of routing protocol instances.";
      list routing-protocol {
        key "type name";
        description
          "State data of a routing protocol instance.

           An implementation MUST provide exactly one
           system-controlled instance of the type 'direct'. Other
           instances MAY be created by configuration.";
        leaf type {
          type identityref {
            base routing-protocol;
          }
          description
            "Type of the routing protocol.";
        }
        leaf name {
          type string;
          description
            "The name of the routing protocol instance.

             For system-controlled instances this name is persistent,
             i.e., it SHOULD NOT change across reboots.";
        }
      }
    }
    container ribs {
      description
        "Container for RIBs.";
      list rib {
        key "name";
        min-elements "1";
        description
          "Each entry represents a RIB identified by the 'name' key.
           All routes in a RIB MUST belong to the same address
           family.

           An implementation SHOULD provide one system-controlled
           default RIB for each supported address family.";
        leaf name {
          type string;
          description
            "The name of the RIB.";
        }
        uses address-family;
        leaf default-rib {
          if-feature multiple-ribs;
          type boolean;
          default "true";
          description
            "This flag has the value of 'true' if and only if the RIB
             is the default RIB for the given address family.

             A default RIB always receives direct routes. By default
             it also receives routes from all routing protocols.";
        }
        container routes {
          description
            "Current content of the RIB.";
          list route {
            description
              "A RIB route entry. This data node MUST be augmented
               with information specific for routes of each address
               family.";
            leaf route-preference {
              type route-preference;
              description
                "This route attribute, also known as administrative
                 distance, allows for selecting the preferred route
                 among routes with the same destination prefix. A
                 smaller value means a more preferred route.";
            }
            container next-hop {
              description
                "Route's next-hop attribute.";
              uses next-hop-state-content;
            }
            uses route-metadata;
          }
        }
      }
    }
  }

  /* Configuration Data */

  container routing {
    description
      "Configuration parameters for the routing subsystem.";
    uses router-id {
      if-feature router-id;
      description
        "Configuration of the global router ID. Routing protocols
         that use router ID can use this parameter or override it
         with another value.";
    }
    container routing-protocols {
      description
        "Configuration of routing protocol instances.";
      list routing-protocol {
        key "type name";
        description
          "Each entry contains configuration of a routing protocol
           instance.";
        leaf type {
          type identityref {
            base routing-protocol;
          }
          description
            "Type of the routing protocol - an identity derived from
             the 'routing-protocol' base identity.";
        }
        leaf name {
          type string;
          description
            "An arbitrary name of the routing protocol instance.";
        }
        leaf description {
          type string;
          description
            "Textual description of the routing protocol instance.";
        }
        container static-routes {
          when "../type='rt:static'" {
            description
              "This container is only valid for the 'static' routing
               protocol.";
          }
          description
            "Configuration of the 'static' pseudo-protocol.

             Address-family-specific modules augment this node with
             their lists of routes.";
        }
      }
    }
    container ribs {
      description
        "Configuration of RIBs.";
      list rib {
        key "name";
        description
          "Each entry contains configuration for a RIB identified by
           the 'name' key.

           Entries having the same key as a system-controlled entry
           of the list /routing-state/ribs/rib are used for
           configuring parameters of that entry. Other entries define
           additional user-controlled RIBs.";
        leaf name {
          type string;
          description
            "The name of the RIB.

             For system-controlled entries, the value of this leaf
             must be the same as the name of the corresponding entry
             in state data.

             For user-controlled entries, an arbitrary name can be
             used.";
        }
        uses address-family {
          description
            "Address family of the RIB.

             It is mandatory for user-controlled RIBs. For
             system-controlled RIBs it can be omitted, otherwise it
             must match the address family of the corresponding state
             entry.";
          refine "address-family" {
            mandatory "false";
          }
        }
        leaf description {
          type string;
          description
            "Textual description of the RIB.";
        }
      }
    }
  }

  /* RPC operations */

  rpc fib-route {
    description
      "Return the active FIB route that is used for sending packets
       to a destination address.";
    input {
      container destination-address {
        description
          "Network layer destination address.

           Address family specific modules MUST augment this
           container with a leaf named 'address'.";
        uses address-family;
      }
    }
    output {
      container route {
        description
          "The active FIB route for the specified destination.

           If no active FIB route exists for the destination address,
           no output is returned - the server SHALL send an
           <rpc-reply> containing a single element <ok>.

           Address family specific modules MUST augment this list
           with appropriate route contents.";
        uses address-family;
        container next-hop {
          description
            "Route's next-hop attribute.";
          uses next-hop-state-content;
        }
        uses route-metadata;
      }
    }
  }
}

@woytowg
Copy link
Author

woytowg commented Apr 24, 2019

Sorry, meant to create an attachment...

@michalvasko
Copy link
Member

Hi,
the grouping next-hop-content is not instiantiated in the model, are you importing it in your own model, can you also provide that one?

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented Apr 25, 2019

@michalvasko
Copy link
Member

Hi,
the modules you provided are a mess, the node with the supposed issue is even commented out. I believe there is a problem but if I am will not be able to reproduce it, there is no way for me to fix it.

I have just now noticed in the backtrace that the problem occurs when state data are being freed, so the problematic node would then be next-hop-address in augment "/rt:routing-state/rt:ribs/rt:rib/rt:routes/rt:route/rt:next-hop/rt:next-hop-options". I tried to modify the models so they work together and create this node, but it was freed without problems. So, if you could provide me with the actual models you have loaded into sysrepo and the freed data tree (you can see it if you, for example, put printer before freeing the nodes in dm_remove_added_data_trees) then I will be happy to fix the issue. Otherwise I am not able to.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented Apr 25, 2019

I agree the models are a mess, we have wanted to update for a while but have not had the opportunity, but believe it or not those are the actual models we are using... anyway....

Could you send me exactly what printing/logging you want me to add to the dm_remove_added_data_trees function? Would this be sufficient?

    while (n) {
       tmp = n;
       n = n->next;
       if (module != tmp->schema->module) {
          SR_LOG_ERR("before lyd_free (%s:%s).",tmp->schema->module->name, tmp->schema->module->rev_size ? tmp->schema->module->rev[0].date : "");
          lyd_free(tmp);
       }
    }

@michalvasko
Copy link
Member

Hi,
just putting lyd_print_file(stdout, data_info->node, LYD_XML, LYP_FORMAT | LYP_WITHSIBLINGS); at data_manager.c:2202 should do the trick. There can possibly be a lot of output but only the last one before the error will be relevant. Thanks.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented May 1, 2019

The lyd_print_file is being called but I don't see the output anywhere... sysrepod is running as a systemd service, not sure if that's the problem or not, any suggestions? otherwise I will keep trying to get the printout to work somehow, maybe use a actual file...

@michalvasko
Copy link
Member

Hi,
right, it would not work if daemonized (not using -d). So yes, you can print it into a file (lyd_print_path), that may be even better because it will be overwritten and only the last output will remain.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented May 6, 2019

I just updated to version d1ad4ee and I am no longer seeing the errors!!

@woytowg
Copy link
Author

woytowg commented May 6, 2019

When I was debugging I noticed that ipv4-address and ipv4-address-no-zone were missing from the array shown below. This change is not required to fix this issue, but just wanted to mention it in case you think the change should be made anyway.

diff --git a/src/user_types/user_inet_types.c b/src/user_types/user_inet_types.c

index 64ec28ea..85769211 100644
--- a/src/user_types/user_inet_types.c
+++ b/src/user_types/user_inet_types.c
@@ -295,8 +295,10 @@ ip_prefix_store_clb(struct ly_ctx *ctx, const char *type_name, const char **valu
 /* Name of this array must match the file name! */
 struct lytype_plugin_list user_inet_types[] = {
     {"ietf-inet-types", "2013-07-15", "ip-address", ip_store_clb, NULL},
+    {"ietf-inet-types", "2013-07-15", "ipv4-address", ip_store_clb, NULL},**
     {"ietf-inet-types", "2013-07-15", "ipv6-address", ip_store_clb, NULL},
     {"ietf-inet-types", "2013-07-15", "ip-address-no-zone", ip_store_clb, NULL},
+    {"ietf-inet-types", "2013-07-15", "ipv4-address-no-zone", ip_store_clb, NULL},**
     {"ietf-inet-types", "2013-07-15", "ipv6-address-no-zone", ip_store_clb, NULL},
     {"ietf-inet-types", "2013-07-15", "ip-prefix", ip_prefix_store_clb, NULL},
     {"ietf-inet-types", "2013-07-15", "ipv4-prefix", ipv4_prefix_store_clb, NULL},

@michalvasko
Copy link
Member

Hi,
IPv4 addresses are not included on purpose, they are always in their canonical form.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented May 7, 2019

I'm not sure why but after updating libyang, libnetconf2, sysrepo, and Netopeer2 to latest and rebuilding everything I'm getting the errors again :-(

@woytowg
Copy link
Author

woytowg commented May 7, 2019

Here's the lyd_print_file print just before the error
node.xml.txt

@michalvasko
Copy link
Member

Hi,
okay, could you also please provide the YANG module with namespace http://vcinity.io/radx/interfaces? If it is private, you could send it to my email only.

Regards,
Michal

@michalvasko
Copy link
Member

Hi,
thanks for the effort but I failed to reproduce the error, sadly. But, you can debug it so we try it that way. Firstly, please print all 3 parameters of lytype_free() right when the function is called because type is changed afterwards. Also, are you using any custom user_types plugins?

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented May 13, 2019

I pulled the latest libyang changes and I think the "Internal error" has gone away... need to do some more testing to be sure. Would you expect any recent changes to have an effect on this?

@michalvasko
Copy link
Member

Hi,
yes, the latest change b4f59e9 did fix an internal error in plugins.c but the message was on a different line so I was not sure whether it also fixed this issue or not, it is definitely possible.

Regards,
Michal

@woytowg
Copy link
Author

woytowg commented May 13, 2019

Great!! I still have some more tests to perform but so far so good. I should be able to finish testing by the end of the day today and will let you know. Thanks!!

@woytowg
Copy link
Author

woytowg commented May 14, 2019

FYI - I'm not seeing this error anymore but I am getting memory corruption crashes (invalid free etc) in my daemons. I'm not sure if this is from the sysrepo/libyang updates or from my daemon changes, it will take a bit for me to figure this out. I will let you know either way.

@woytowg
Copy link
Author

woytowg commented May 14, 2019

All is good, no libyang errors and fixed the crash bugs in my software. thx!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants